Behaviour of istream_iterator on closed stream
Old Wolf
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.
--
Ian Collins
Alf P. Steinbach
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
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?
--
Ian Collins
James Kanze
> > 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.
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.
> > 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?
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.)
--
James Kanze (GABI Software) email:james...@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
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:
> > Old Wolf wrote:
> >> 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.
> 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
> On Jul 29, 10:00 pm, James Kanze <james.ka...@gmail.com> wrote:
>> 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!
This might help next time :
http://www.parashift.com/c++-faq-lite/how-to-post.html#faq-5.8
James Kanze
Concepts. But they've been dropped from C++0x.
Pete Becker
> On Jul 29, 12:13 pm, Old Wolf <oldw...@inspire.net.nz> wrote:
>
>> Hopefully something in C++1x would help the compiler to
>> diagnose this bug..
>
> Concepts. But they've been dropped from C++0x.
>
But they will be in C++1x. <g>
--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of
"The Standard C++ Library Extensions: a Tutorial and Reference"
(www.petebecker.com/tr1book)