Behaviour of istream_iterator on closed stream

[Old Wolf]

Code snippet:

std::ifstream file( "does_not_exist" );
std::vector<char> file_vec;
std::copy( std::istream_iterator<char>(file),
std::istream_iterator<char>(), std::back_inserter(file_vec) );
if ( file_vec.empty() )
// file did not exist or had no contents..


I expected this to work, and have the istream_iterator for
closed file return a blank iterator, so the copy would not
copy anything. But it segfaults. Is the behaviour
undefined or is my compiler broken?

 

[Ian Collins]

Code snippet:

std::ifstream file( "does_not_exist" );

You should have tested the state of file before going any further. It's
a pity this std::ifstream constructor doesn't throw an exception.

std::vector<char> file_vec;
std::copy( std::istream_iterator<char>(file),
std::istream_iterator<char>(), std::back_inserter(file_vec) );
if ( file_vec.empty() )
// file did not exist or had no contents..


I expected this to work, and have the istream_iterator for
closed file return a blank iterator, so the copy would not
copy anything. But it segfaults. Is the behaviour
undefined or is my compiler broken?

I think all bets are off, the std::ifstream object isn't fully initialised.

 

[Alf P. Steinbach]

* Ian Collins:

You should have tested the state of file before going any further. It's
a pity this std::ifstream constructor doesn't throw an exception.


I think all bets are off, the std::ifstream object isn't fully initialised.

Hm. It is perhaps well known to you that I'm not very enthusiastic about
iostreams, but I find that hard to believe. Is it really true?

Yeah, I remember something about internal two-phase construction with an Init_
call or something, very bad.

But still -- how could one check for success if the object isn't fully
initialized?


Cheers,

- Alf

 

[Ian Collins]

* Ian Collins:

Hm. It is perhaps well known to you that I'm not very enthusiastic about
iostreams, but I find that hard to believe. Is it really true?

Yeah, I remember something about internal two-phase construction with an
Init_ call or something, very bad.

Which is why I think it should throw.

But still -- how could one check for success if the object isn't fully
initialized?

A flag initialised to false and set true once the object is built?

 

[James Kanze]

You should have tested the state of file before going any
further. It's a pity this std::ifstream constructor doesn't
throw an exception.

That's very discutable. You have to check the status anyway,
after every read, so there's no invariant that you haven't been
able to establish.

Either your compiler is broken, or (more likely) you have
undefined behavior elsewhere which has corrupted memory, and
caused it to fail here.

Try creating a minimum sized program which displays the error.
(If you do nothing but the above in main, and it crashes, you
do have a compiler problem. It works with g++, Sun CC and VC++,
however.)

I think all bets are off, the std::ifstream object isn't fully
initialised.

If you return from the constructor, the object is fully
initialized. By definition, for starters---what doesn't take
place in the constructor isn't considered initialization. But
in the case of std::ifstream (and all of the object types
defined in the standard), the constructor is required to ensure
that the object is usable as usual. (I'd still check for an
error after construction, but before the copy, since the type of
error you want to report will be different if the file doesn't
exist.)

 

[Old Wolf]

Either your compiler is broken, or (more likely) you have
undefined behavior elsewhere which has corrupted memory, and
caused it to fail here.

You're right, in code immediately following this, for
the case where no characters were read, I had some
bad code. But I did change the said code to test if
the file opened successfully just to be on the safe side!

The buggy code was like this:

void foo( unsigned char *buffer, size_t buf_len )
{
std::fill_n(buffer, buffer + buf_len, 0);
}

Obviously it should be fill instead of fill_n. But the
compiler did not give a warning. I would have expected
a warning from passing a pointer for the second argument
instead of a count. But, fill_n is implemented as a
template function with all parameter types as template
parameters. So fill_n looped, decrementing the second
argument until it compared equal to 0!

Hopefully something in C++1x would help the compiler
to diagnose this bug..

 

[James Kanze]

* Ian Collins:

Hm. It is perhaps well known to you that I'm not very
enthusiastic about iostreams, but I find that hard to believe.
Is it really true?

No. I don't know where Ian got that (he's usually pretty
reliable about such things). The object is, in fact, fully
constructed before it tries to open the file; if the open fails,
failbit is set. There's also a separate flag is_open, which
would allow testing after the copy.

Yeah, I remember something about internal two-phase
construction with an Init_ call or something, very bad.

There's no two-phase construction for the complete objects.
There is a slight issue concerning construction of the lowest
base class; because inheritance is virtual (since iostream must
derive from istream and ostream, which both share a common base
class), the lowest class must be initialized by the most derived
class. Which potentially doesn't have all of the necessary
information to do so. So the default constructor of the base
class doesn't actually initialize; one of the derived classes is
required to call an init() function later. (And I agree that
this is a bad solution---the default constructor could have
initialized to an error state, which could be overridden by the
init function.)

 

[Vladimir Jovic]

You're right, in code immediately following this, for
the case where no characters were read, I had some
bad code. But I did change the said code to test if
the file opened successfully just to be on the safe side!

This might help next time :
http://www.parashift.com/c++-faq-lite/how-to-post.html#faq-5.8

 

[James Kanze]

You're right, in code immediately following this, for the case
where no characters were read, I had some bad code. But I did
change the said code to test if the file opened successfully
just to be on the safe side!

The buggy code was like this:

void foo( unsigned char *buffer, size_t buf_len )
{
std::fill_n(buffer, buffer + buf_len, 0);
}

Obviously it should be fill instead of fill_n. But the
compiler did not give a warning. I would have expected a
warning from passing a pointer for the second argument instead
of a count. But, fill_n is implemented as a template function
with all parameter types as template parameters. So fill_n
looped, decrementing the second argument until it compared
equal to 0!

Hopefully something in C++1x would help the compiler to
diagnose this bug..

Concepts. But they've been dropped from C++0x.