Several ifstreams access-violation

Question

I try to realise an external merge sort (wiki) and I want to open 2048 ifstreams and read data to personal buffers.

ifstream *file;
file = (ifstream *)malloc(2048 * sizeof(ifstream));

for (short i = 0; i < 2048; i++) {
    itoa(i, fileName + 5, 10);
    file[i].open(fileName, ios::in | ios::binary); // Access violation Error
    if (!file[i]) {
        cout << i << ".Bad open file" << endl;          
    }
    if (!file[i].read((char*)perfile[i], 128*4)) {
        cout << i << ". Bad read source file" << endl;
    }       
}

But, it crashes with

Unhandled exception at 0x58f3a5fd (msvcp100d.dll) in sorting.exe: 0xC0000005: Access violation reading location 0xcdcdcdfd.

Is it possible to use so much opened ifstreams? Or maybe it is very bad idea to have 2048 opened ifstreams and there is a better way to realize this algorithm?

It could be that since you `malloc` and not `new`, the constructors of `ifstream`s are not called, so there's a problem there. — Shahbaz, Nov 16 '11 at 23:35

score 4 · Answer 1 · answered Nov 16 '11 at 23:32

This is C++.ifstream is non-POD, so you can't just malloc it: the instances need to get constructed

ifstream file[2048];

for (short i = 0; i < 2048; i++) {
    itoa(i, fileName + 5, 10);
    file[i].open(fileName, ios::in | ios::binary); // Access violation Error
    if (!file[i]) {
        cout << i << ".Bad open file" << endl;          
    }
    if (!file[i].read((char*)perfile[i], 128*4)) {
        cout << i << ". Bad read source file" << endl;
    }       
}

Besides that, opening 2048 files doesn't sound like a good plan, but you can figure that out later

Ed S. · Answer 2 · 2011-11-16T23:39:06.270

4

The value 0xcdcdcdcd is used by VS in debug mode to represent uninitialized memory (also keep an eye out for 0xbaadf00d).

You are using malloc which is of C heritage and does not call constructors, it simply gives you a pointer to a chunk of data. An ifstream is not a POD (Plain Old Data) type; it needs you to call its constructor in order to initialize properly. This is C++; use new and delete.

Better yet, don't use either; just construct the thing on the stack and let it handle dynamic memory allocation as it was meant to be used.

Of course, this doesn't even touch on the horrible idea to open 2048 files, but you should probably learn that one the hard way...

edited Nov 16 '11 at 23:39

answered Nov 16 '11 at 23:34

Ed S.

122,712
22
185
265

Thank you. It seems like OS don't allow to open so much files -- I used new, but on i == 508 an open file error is occured. So... what is the better way for do it? Just open, read and close? – 3ka5_cat Nov 16 '11 at 23:46
@Shahbaz: IIRC 0xcdcdcdcd is used by the VC++ debug heap, while 0xbaadf00d is used by the Windows debug heap (that is activated whenever a process is started with a debugger attached). – Matteo Italia Nov 16 '11 at 23:46
@user1050686: File handles are a limited resource. You should close each file after you're done with it. You don't need an array; you need one scoped `ifstream`. It will be closed when the scope is left, i.e., declare the variable (stack allocated!) in your for loop. Look up RAII and [read this](http://stackoverflow.com/questions/748014/do-i-need-to-manually-close-a-ifstream). – Ed S. Nov 16 '11 at 23:50

Matteo Italia · Accepted Answer · 2011-11-16T23:51:33.507

4

Arrays of non-POD objects are allocated with new, not with malloc, otherwise the constructors aren't run.

Your code is getting uninitialized memory and "interpreting" it as ifstreams, which obviously results in a crash (because the constructor of the class hasn't been run not even the virtual table pointers are in place).

You can either allocate all your objects on the stack:

ifstream file[2048];

or allocate them on the heap if stack occupation is a concern;

ifstream *file=new ifstream[2048];
// ...
delete[] file; // frees the array

(although you should use a smart pointer here to avoid memory leaks in case of exceptions)

or, better, use a vector of ifstream (requires header <vector>):

vector<ifstream> file(2048);

which do not require explicit deallocation of its elements.

(in theory, you could use malloc and then use placement new, but I wouldn't recommend it at all)

... besides, opening 2048 files at the same time doesn't feel like a great idea...

edited Nov 16 '11 at 23:51

answered Nov 16 '11 at 23:34

Matteo Italia

123,740
17
206
299

1

@Shahbaz: one little (justified) rant is enough to kill an otherwise complete (and I hope correct) answer? – Matteo Italia Nov 16 '11 at 23:38
3

Well, there's no reason to go out of your way to pounce on an obvious beginner. – Ed S. Nov 16 '11 at 23:42
Done. I hope you'll like it more now. – Matteo Italia Nov 16 '11 at 23:45
1

I removed my -1. If you are here to help, try to be a kind teacher, rather than an angry college senior. – Shahbaz Nov 16 '11 at 23:56

score 0 · Answer 4 · answered Nov 16 '11 at 23:31

0

You cannot open 2048 files, there is an operating system limit for open files

answered Nov 16 '11 at 23:31

Daniel

30,896
18
85
139

That may be true, but definitely **not** the problem – sehe Nov 16 '11 at 23:32
@sehe: you are right, its about the non constructed objects. I will keep this as a possible another problem – Daniel Nov 16 '11 at 23:36

score 0 · Answer 5 · answered Nov 16 '11 at 23:50

As far as I can see, you don't really need an array of 2048 separate ifstreams here at all. You only need one ifstream at any given time, so each iteration you close one file and open another. Destroying an ifstream closes the file automatically, so you can do something like this:

for (short i = 0; i < 2048; i++) {
    itoa(i, fileName + 5, 10);
    ifstream file(fileName, ios::in | ios::binary);
    if (!file) {
        cout << i << ".Bad open file" << endl;          
    }
    if (!file.read((char*)perfile[i], 128*4)) {
        cout << i << ". Bad read source file" << endl;
    }       
}

Several ifstreams access-violation

5 Answers5