STL vector<float> crash

[Active8]

I put the bare essentials in a console app.

http://home.earthlink.net/~mcolasono/tmp/degub.zip

Opening output.fft and loading it into a vector<float> screws
up, but input1.dat doesn't. It does load the vector, too. It just
craps out when you return to the console, or in a [wx]Windows app,
the message loop. I think it has something to do with the
vector<float> going out of scope and trying to free memory, but
don't know why it would do that. The fft file is 512 floats and I
converted the actual values (which came out of the vector which
crashed, BTW) to test.txt.

I did it in VC++. Maybe another compiler or OS won't have a prob
with it.

I'd appreciate it if someone could try it out and see what happens.

Thanks in advance.

 

[Dietmar Kuehl]

I put the bare essentials in a console app.

Actually, you didn't! This code uses at least one [actually even
unnecessary], environment specific header and I doubt that you
have reduced it to the bare minimum of code exposting the problem.
You are using the wrong convention for headers (you shall not use
angle brackets for your headers; angle brackets are reserved for
the system, i.e. the standard library headers), you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'), and you don't
check for any error codes!

A minimal example typically takes something like 30 lines of code.
In reducing the code to a minimal amount you would normally also
stumble over some thinko...

 

[Andrew Koenig]

you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'

....umm, aren't you forgetting about the compatibility hack that permits a
string literal to decay to char*?

 

[Active8]

you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'

...umm, aren't you forgetting about the compatibility hack that permits a
string literal to decay to char*?

I've seen plenty of code that passes literals that way. I started
the <> includes when I put the files in a separate dir and meant to
change those to quotes. If I've read that <> is reserved for system
files, I forgot, but now I know.

 

[Active8]

I put the bare essentials in a console app.

Actually, you didn't! This code uses at least one [actually even
unnecessary], environment specific header and I doubt that you

What file would that be? stdio.h?

have reduced it to the bare minimum of code exposting the problem.

The dspfile class could've been stripped dow a bit, but it's so
small anyway...

You are using the wrong convention for headers (you shall not use
angle brackets for your headers; angle brackets are reserved for
the system, i.e. the standard library headers), you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'), and you don't
check for any error codes!

A minimal example typically takes something like 30 lines of code.
In reducing the code to a minimal amount you would normally also
stumble over some thinko...

Does all that make it so hard to compile and run it?

 

[Dietmar Kuehl]

What file would that be? stdio.h?

windows.h as used in dspfile.cpp.

The dspfile class could've been stripped dow a bit, but it's so
small anyway...

There are nearly 500 lines. Typical example programs are 20 to 30
lines; rather big examples, commented in the text body of the
message, are typically still less than 100 lines.

Does all that make it so hard to compile and run it?

Yes, it does: it took me about 10 minutes before I got an executable
build. This executable immediately crashed when called without
arguments, i.e. a simple sanity check for the executable already
failed. Contrast this with maybe 40 lines of code I cut and paste
from the browser into a C++ file and which runs mere seconds later!

 

[Dietmar Kuehl]

you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'

...umm, aren't you forgetting about the compatibility hack that permits a
string literal to decay to char*?

No, I'm not! Maybe it is just me but I tend to use highest warning
level and remove at least most warnings before even considering to run
the code. This normally catches most problems. Using 'char const*'
instead of 'char*' is a trivial change and does not impose any new
constraints (writing to the string literal is prohibited even if it
declared as 'char*'; only the compiler cannot catch the errors related
to writing to string literals). A similar argument applies to the
things I mentioned. Of course, I can make <> delimited, user-defined
headers to work; it just takes another compilation cycle and editing
the compiler flags but should this really be necessary? Of course, I
can figure out how to call the program from the source rather than by
trying to just run it to see that there is yet another problem lurking.

I don't mind to help others as you are probably well aware of. However,
I generally assume that the others have made a reasonable attempt to
solve their problem first. I cannot see an indication of this here.

 

[Dietmar Kuehl]

I've seen plenty of code that passes literals that way.

.... and all of this code should be flagged with a warning! It is an
error prone habit since you cannot write to string literals but the
compiler cannot help you by prohiting it, i.e. the error occurs at
run-time. The fact there is plenty of code out there meant that a
deprecated features was added to C++ but no new code shall rely on
this feature!

 

[E. Mark Ping]

Opening output.fft and loading it into a vector<float> screws
up, but input1.dat doesn't.

Your "load_vector" function is tremendously unsafe. You resize the
vector based on the leader bytes of the file, but there is no check
during the load if you go off the end. (IME, writing past the end of
the vector is the easiest way to crash.)

Furthermore, your call:
fread(it++, sizeof(vec_type), 1, m_channel);

is problematic to say the least. "it" is a T::iterator, which is not
guaranteed at all to be a pointer. At least, you should use:

fread(&(*it++), sizeof(vec_type), 1, m_channel);

And if you're using vector, you could do:

itemCount = vec.size();
fread(&vec[0], sizeof(vec_type), itemCount, m_channel);

or something like that.


Have you verified that 'output.fft' has the right amount of data?

 

[assaarpa]

I've seen plenty of code that passes literals that way. I started

the <> includes when I put the files in a separate dir and meant to
change those to quotes. If I've read that <> is reserved for system
files, I forgot, but now I know.

I'd double-check on that.

 

[Old Wolf]

you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'

...umm, aren't you forgetting about the compatibility hack that permits a
string literal to decay to char*?

I think the string literal always decays to (char const *), but
it is permitted to assign one of those to a (char *) if it has
come from decay of a string literal.

 

[Active8]

... and all of this code should be flagged with a warning! It is an
error prone habit since you cannot write to string literals but the
compiler cannot help you by prohiting it, i.e. the error occurs at
run-time. The fact there is plenty of code out there meant that a
deprecated features was added to C++ but no new code shall rely on
this feature!

How should I declare it in the future?

 

[Active8]

you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'

...umm, aren't you forgetting about the compatibility hack that permits a
string literal to decay to char*?

No, I'm not! Maybe it is just me but I tend to use highest warning
level and remove at least most warnings before even considering to run
the code. This normally catches most problems. Using 'char const*'
instead of 'char*' is a trivial change and does not impose any new
constraints (writing to the string literal is prohibited even if it
declared as 'char*'; only the compiler cannot catch the errors related
to writing to string literals).

Actually, in the app it comes from, I read the filename from a text
control into a string class and pass str.c_str();

A similar argument applies to the
things I mentioned. Of course, I can make <> delimited, user-defined
headers to work; it just takes another compilation cycle and editing
the compiler flags but should this really be necessary? Of course, I
can figure out how to call the program from the source rather than by
trying to just run it to see that there is yet another problem lurking.

I don't mind to help others as you are probably well aware of. However,
I generally assume that the others have made a reasonable attempt to
solve their problem first. I cannot see an indication of this here.

I've spent three days trying things I could think of. Sorry I didn't
whittle it down to your satisfaction.

 

[Active8]

Your "load_vector" function is tremendously unsafe. You resize the
vector based on the leader bytes of the file, but there is no check
during the load if you go off the end. (IME, writing past the end of
the vector is the easiest way to crash.)

Thanks. It's fixed per one of your suggestions.

The vector loaded without crashing. It crashed when it went out of
scope. I also used a message box in the app which reported that the
vector contained 512 valuses. Hmmm.. it's still there but commented
out for the console app.

How would I check going off the end during load other than using
something like i<512 in a for loop instead of the feof() check?

Furthermore, your call:
fread(it++, sizeof(vec_type), 1, m_channel);

is problematic to say the least. "it" is a T::iterator, which is not
guaranteed at all to be a pointer. At least, you should use:

fread(&(*it++), sizeof(vec_type), 1, m_channel);

And if you're using vector, you could do:

itemCount = vec.size();
fread(&vec[0], sizeof(vec_type), itemCount, m_channel);

that's an idea. I didn't know iterators were unsafe. What's the
point in having them?

or something like that.

Have you verified that 'output.fft' has the right amount of data?

Yes. Same number as input1.dat. 512 And there's 512 In test.txt.

I tried your suggestion:

fread(&vec[0], sizeof(vec_type), itemCount, m_channel);

and it seems to work. I can see where using the feof() return to
stop the load isn't a good idea, especially since I'll probably add
trailer text later. Yet I used:

while( it != vec.end() )
fwrite(it++, sizeof(vec_type), 1, m_channel);

For the write function.

As much as I'd like to know *why* it crashed... I bet it was that
feof() thing causing the vector to overload and it just didn't crash
'till it went out of scope.

Thanks, EMP.

 

[E. Mark Ping]

The vector loaded without crashing. It crashed when it went out of
scope.

Which is when the destructor is called. If the state of the vector
gets corrupted, that's where it's likely to crash.

How would I check going off the end during load other than using
something like i<512 in a for loop instead of the feof() check?

One way would be what I suggested below, which is explicitly load the
number of values indicated in the header (and shrink if the actual
number read is less than that). Otherwise you're asking for a buffer
overrun (in fact, I believe precisely this kind of issue was the
source of the MS GDI+ bug).

that's an idea. I didn't know iterators were unsafe. What's the
point in having them?

Iterators aren't unsafe, but they aren't guaranteed to be pointer
types. Hence if you want a real pointer to what 'it' is an iterator
over, you use: &(*it). The operator* may involve more than a simple
pointer dereference (indeed, for reverse iterators that commonly the
case).

I tried your suggestion:

fread(&vec[0], sizeof(vec_type), itemCount, m_channel);

and it seems to work. I can see where using the feof() return to
stop the load isn't a good idea, especially since I'll probably add
trailer text later. Yet I used:

while( it != vec.end() )
fwrite(it++, sizeof(vec_type), 1, m_channel);

For the write function.

That also looks reasonable.

As much as I'd like to know *why* it crashed... I bet it was that
feof() thing causing the vector to overload and it just didn't crash
'till it went out of scope.

Yes, probably it ran off the end of the array, and when the vector
destructor tried to deallocate, the bytes marking the end were trashed
and the allocator barfed (to use technical terms).

Thanks, EMP.

You're welcome.

 

[Dietmar Kuehl]

Actually, in the app it comes from, I read the filename from a text
control into a string class and pass str.c_str();

.... which is, of course, a 'char const*' which cannot be passed
as 'char*', i.e. you the compiler should reject attempts to use
it with your file class. I think it is odd you deliberately broke
your code before posting...

I've spent three days trying things I could think of. Sorry I didn't
whittle it down to your satisfaction.

I haven't inspected your code too closely but if I were in your
position, I would reduce the code until the point it stops crashing!
If you know which statement crashes the program, you almost certainly
have the cause of the problem. Also, by then the problem should have
become much smaller because you probably just need to look at two
dozens of lines rather then twenty times as much.

When asking for help in a voluntary forum I think it is a courtesy
to the reader to pin-point the problem as much as possible and to
use readily compilable and probably warning free code. Still,
I actually already pointed out a definite source of problems: you
don't do any error checking. For example, you don't check whether
a file contains more records than you expect after reading their
size (as also posted out by another reply): apply thorough checking
for unexpected conditions (and messages in case unexpected
conditions arise) and I would guess your solve your problem!

 

[Dietmar Kuehl]

How should I declare it in the future?

Maybe you should read replies more thoroughly? Let me quote myself:

you pass string
literals as 'char*' (you can't: the type of a string literals is
'char const(&)[n]' which decays to a 'char const*'

Note the type I mentioned at the very end of this quote:
'char const*' (or 'const char*'; these are identical but I
consider the rightmost placement more legible).

 

[Active8]

Thanks for the tips.

 

[Karl Heinz Buchegger]

Thanks for the tips.

Just to show you how simple things can be.

Your code:

while( !feof(m_channel) )
{
fread(it++, sizeof(vec_type), 1, m_channel);
}

Modified:

i = 0;
while( !feof(m_channel) )
{
if( i++ >= m_nrecs*m_lrec )
int j = 0;

fread(&(*it++), sizeof(vec_type), 1, m_channel);
}

The idea is to *check* if the vector is not overflowed.
So at all times it must be true, that a running counter
throughout the whole loop has to stay lower then m_nrecs*m_lrec,
since this is the size with which the vector was resized.
The dependent statement in the if, really doesn't matter. It
is just there to have something to put a breakpoint on.

Needless to say, that this breakpoint is actually reached,
when the program is run Your program definitly tries to
read more data items then were reserved in the vector.

Moral of story: When programming, don't trust anything. Don't
assume anything. Instead put in checks, if your assumptions hold!