Hello!
On Wed, Nov 08, 2017 at 12:36:00PM -0800, Volodymyr Sapsai wrote:
> Thanks for the patch, Reimar. Can you please add tests to ensure this
> functionality doesn’t regress? As null character is required by the standard
> (27.7.2.3p21), a good starting point seems to be
> test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
> test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp
New patch attached, though I went the lazy way of just adding one case.
Not sure that's "good enough" - in principle I think it should be.
More tricky would be to add a test for the _LIBCPP_NO_EXCEPTIONS case,
is there any code testing that at all?
> And what about free function std::getline that takes a stream and a string?
> The standard (21.4.8.9p7) specifies
[...]
>
> Technically, string is not a character array of non-zero size. But according
> to the spirit of the standard, I would expect string to be empty after
> reading into it from a stream that reached EOF. What do you think?
Starting with the useful answer instead of what I think:
I'd try what other implementations do and align with that.
At least libstdc++ does not clear the string, so tentatively
I'd suggest not changing behaviour, for simple interoperability.
Testing at least Microsoft C++ in addition might be a good idea
though...
Test code:
#include <iostream>
#include <sstream>
#include <string>
int main()
{
std::istringstream in;
std::string dummy;
in >> dummy;
char test[20] = "oldstring";
in.getline(test, sizeof(test));
std::cout << "std::istream::getline result: " << test << std::endl;
dummy = "oldstring";
std::getline(in, dummy);
std::cout << "std::getline result: " << dummy << std::endl;
return test[0];
}
As to what I really think, for anyone who isn't tired of unfair rants
(anyone else please just skip):
I don't really care much about that case, because at least unlike
not 0-terminating a char buffer it is not in the "things you never EVER
do" list.
But this rather needs to be put to the standard authors and it needs to be
clarified.
Along with the suggestion to not only spend time on new features but
also on improving the spec quality.
Large parts of the spec read like when a second-semester maths student
tries to write a proof: the language lacks the precision to actually
write a solid proof, and excessive verbosity is used to try to make
up for it (to no effect besides annoying the reader).
The lack of specified pre- and post-conditions (specified formally
or in natural language) for at least parts easy to specify in such a way
to allow at least partial formal verification or proof of correctness
isn't exactly state-of-the-art either.
Kind regards,
Reimar Döffinger
> > On Oct 4, 2017, at 12:07, Reimar Döffinger via cfe-commits
> > <cfe-commits@lists.llvm.org> wrote:
> >
> > If the sentinel failed (e.g. due to having reached
> > EOF before) or an exception was caught it failed to
> > do that.
> > The C++14 standard says:
> > "In any case, if n is greater than zero, it then stores
> > a null character (using charT()) into the next
> > successive location of the array."
> > Other implementations like libstdc++ do 0-terminate and
> > not doing so may lead security issues in applications.
> > ---
> > include/istream | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/istream b/include/istream
> > index 0b8e05d95..5c73df38f 100644
> > --- a/include/istream
> > +++ b/include/istream
> > @@ -1069,16 +1069,18 @@ basic_istream<_CharT, _Traits>::getline(char_type*
> > __s, streamsize __n, char_typ
> > this->rdbuf()->sbumpc();
> > ++__gc_;
> > }
> > - if (__n > 0)
> > - *__s = char_type();
> > if (__gc_ == 0)
> > __err |= ios_base::failbit;
> > this->setstate(__err);
> > }
> > + if (__n > 0)
> > + *__s = char_type();
> > #ifndef _LIBCPP_NO_EXCEPTIONS
> > }
> > catch (...)
> > {
> > + if (__n > 0)
> > + *__s = char_type();
> > this->__set_badbit_and_consider_rethrow();
> > }
> > #endif // _LIBCPP_NO_EXCEPTIONS
> > --
> > 2.14.2
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>From 67ecbad84c70a611cb933b90bb10a10e5f32d4a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <reimar.doeffin...@gmx.de>
Date: Thu, 7 Sep 2017 08:42:10 +0200
Subject: [PATCH] Ensure std::istream::getline always 0-terminates string.
If the sentinel failed (e.g. due to having reached
EOF before) or an exception was caught it failed to
do that.
The C++14 standard says:
"In any case, if n is greater than zero, it then stores
a null character (using charT()) into the next
successive location of the array."
Other implementations like libstdc++ do 0-terminate and
not doing so may lead security issues in applications.
---
include/istream | 6 ++++--
.../istream.unformatted/getline_pointer_size.pass.cpp | 7 +++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/istream b/include/istream
index 0b8e05d95..5c73df38f 100644
--- a/include/istream
+++ b/include/istream
@@ -1069,16 +1069,18 @@ basic_istream<_CharT, _Traits>::getline(char_type* __s, streamsize __n, char_typ
this->rdbuf()->sbumpc();
++__gc_;
}
- if (__n > 0)
- *__s = char_type();
if (__gc_ == 0)
__err |= ios_base::failbit;
this->setstate(__err);
}
+ if (__n > 0)
+ *__s = char_type();
#ifndef _LIBCPP_NO_EXCEPTIONS
}
catch (...)
{
+ if (__n > 0)
+ *__s = char_type();
this->__set_badbit_and_consider_rethrow();
}
#endif // _LIBCPP_NO_EXCEPTIONS
diff --git a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
index 465824a65..6bf917fb4 100644
--- a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
+++ b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
@@ -59,6 +59,13 @@ int main()
assert(!is.fail());
assert(std::string(s) == " ");
assert(is.gcount() == 1);
+ is.getline(s, 5);
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert( is.eof());
+ assert( is.fail());
+ assert(std::string(s) == "");
+ assert(is.gcount() == 0);
}
{
testbuf<wchar_t> sb(L" \n \n ");
--
2.15.0
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
