undefined behaviour in tuple_io.hpp (bug)(patch)
In the tuple_io.hpp header, there seems to be a slight problem in extract_and_check_delimiter(). The following snippet is problematic: char c; if (is_delimiter) { is >> c; if (c!=d) { is.setstate(std::ios::failbit); } } If c cannot be read because is.eof(), the result of c!=d is undefined. A simple fix is attached. cheers, Rupert -- Rupert Kittinger <rkit@mur.at> *** tuple_io.hpp.orig Wed Aug 24 21:05:26 2005 --- tuple_io.hpp Wed Aug 24 21:07:13 2005 *************** *** 349,355 **** char c; if (is_delimiter) { is >> c; ! if (c!=d) { is.setstate(std::ios::failbit); } } --- 349,355 ---- char c; if (is_delimiter) { is >> c; ! if (is.good() && c!=d) { is.setstate(std::ios::failbit); } } *************** *** 443,449 **** CharType c; if (is_delimiter) { is >> c; ! if (c!=d) { is.setstate(std::ios::failbit); } } --- 443,449 ---- CharType c; if (is_delimiter) { is >> c; ! if (is.good() && c!=d) { is.setstate(std::ios::failbit); } }
Rupert Kittinger wrote:
In the tuple_io.hpp header, there seems to be a slight problem in extract_and_check_delimiter(). The following snippet is problematic:
char c; if (is_delimiter) { is >> c; if (c!=d) { is.setstate(std::ios::failbit); } }
If c cannot be read because is.eof(), the result of c!=d is undefined.
IMHO the fix is neater as: if (is>>c && c!=d) basic_ios::operator void*() exists to support this syntax rather than explicitly testing the state with good(), fail() etc. jon
Jonathan Wakely wrote:
Rupert Kittinger wrote:
In the tuple_io.hpp header, there seems to be a slight problem in extract_and_check_delimiter(). The following snippet is problematic:
char c; if (is_delimiter) { is >> c; if (c!=d) { is.setstate(std::ios::failbit); } }
If c cannot be read because is.eof(), the result of c!=d is undefined.
IMHO the fix is neater as:
if (is>>c && c!=d)
basic_ios::operator void*() exists to support this syntax rather than explicitly testing the state with good(), fail() etc.
jon
Using is.good() is consistent with the rest of the file. Another fine point that is not visible from the context diff is that the code works as expected, because the only visible effect of the function is the change in stream state. If is >> c failed because of is.eof(), the failbit is already set, so it is irrelevant whether the if block is executed. But I get false positives from valgrind in my unit tests, and in this kind of code, the stack trace looks somewhat confusing :-) cheers, Rupert -- Rupert Kittinger <rkit@mur.at>
On Aug 25, 2005, at 1:20 PM, Rupert Kittinger wrote:
Jonathan Wakely wrote:
Rupert Kittinger wrote:
In the tuple_io.hpp header, there seems to be a slight problem in extract_and_check_delimiter(). The following snippet is problematic:
char c; if (is_delimiter) { is >> c; if (c!=d) { is.setstate(std::ios::failbit); } }
If c cannot be read because is.eof(), the result of c!=d is undefined.
IMHO the fix is neater as:
if (is>>c && c!=d)
basic_ios::operator void*() exists to support this syntax rather than explicitly testing the state with good(), fail() etc.
jon
Using is.good() is consistent with the rest of the file.
Another fine point that is not visible from the context diff is that the code works as expected, because the only visible effect of the function is the change in stream state. If is >> c failed because of is.eof(), the failbit is already set, so it is irrelevant whether the if block is executed. But I get false positives from valgrind in my unit tests, and in this kind of code, the stack trace looks somewhat confusing :-)
I applied the original patch, thanks Rupert! Jaakko
participants (3)
-
jarvi -
Jonathan Wakely -
Rupert Kittinger