[Bug libstdc++/71950] std::ios_base::failure.what() returns irrelevant error message

[skaianhero at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71950

Austin Kramer  changed:

   What|Removed |Added

 CC||skaianhero at gmail dot com

--- Comment #2 from Austin Kramer  ---
Well, if the actual throw happens obliviously in basic_ios::clear after
fstream::open detects the error, it wouldn't be hard to have fstream::open
catch the exception and re-throw a more helpful one. The real hard part though
is propagating the error details up to fstream::open in the first place. The
call stack goes through several basic succeed/fail returns, some of which are
public-facing functions.

There are 2 reasonable solutions I can think of that don't break the API, but
neither are particularly clean.

 - Gratuitous overloading: Replace some intermediate basic_filebuf and
__basic_file calls with ones that propagate an error code, and have their
public-facing variants wrap the new versions and reduce the return value to the
compliant types.

 - Just use errno: AFAIK there's no spec saying that errno should be set in a
fstream::open call (from the perspective of the caller), but in practice it
seems to be set to the correct value for the underlying I/O error (though this
may be platform-dependent). If g++ could internally require errno to be set
deeper within the file operations and remain set until control returns to
fstream's functions, then those functions could re-throw a fresh exception
after basic_ios::clear with a more helpful message. But like you said, it's
hard (and perhaps wrong) to fully ensure that all sources of errors set errno
appropriately.

That said, I'm not sure how, in a world without PR 66145 causing issues, the
C++11 version would cleanly propagate the system_error details. Maybe it just
doesn't, and this fix would be used to address that as well.

[Bug libstdc++/71950] std::ios_base::failure.what() returns irrelevant error message

[skaianhero at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71950

--- Comment #4 from Austin Kramer  ---
(In reply to Jonathan Wakely from comment #3)
> 
> That would add overhead for exception handling and the vast majority of
> users do not use exceptions with fstream.
> 

That overhead would be almost nothing compared to the disk-accessing I/O that
happens deeper under the hood of these functions. It would simply involve
changing fstream::open to something like this (existing syntax tweaked to make
it easier to fit here):

void open(const char* __s, ios_base::openmode __mode) {
if (!_M_filebuf.open(__s, __mode))
this->setstate(ios_base::failbit);
else
try {
this->clear();
} catch (const std::ios::failure &ex) {
char buf[ESTR_BUF_SZ];
strerror_r(errno, buf, sizeof(buf));
throw std::ios::failure(buf);
}
}

I don't condone magic-mumber-sized buffers, and this is an errno example (that
DOES work on my playform), But my point is that the overhead happens only in
the failure case, after the comparatively expensive main-sequence operation,
with hardly any frame state that needs saving, and it only does any actual
throwing or buffer-writing if clear() is set up to throw. So it should be
pretty negligible.

> 
> Users can specialize basic_filebuf so we can't rely on non-standard
> functions.
> 
> I don't think it's worth changing anything here.

I wouldn't be so sure. Non-standard functionality aside, I still think the
exception message ought to be changed. I agree that checking the status bits of
the stream is the conventional way to go, but for some callers, that just ISN'T
ENOUGH INFORMATION. Most file operations can fail for more than one reason, and
having at least the ABILITY to generate a user or developer-readable error
message from standard library functions can be important in some cases.

Because between this vague message and PR 66145 preventing a valid system_error
containing exception from getting generated, there is NO way to determine why
an fstream function failed. And NOBODY likes seeing "Unknown Error".