[PATCH] Ensure std::getline always 0-terminates string.

[Reimar Döffinger via cfe-commits]

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 at 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 at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Ensure-std-istream-getline-always-0-terminates-strin.patch
Type: text/x-diff
Size: 2538 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171109/adaae0ac/attachment.patch>