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

Clang Frontend
1

If the sentinel failed (e.g. due to having reached
EOF before) or an exception was caught it failed to
do that.
While it seems (unfortunately!) not required by the
specification, libstdc++ does 0-terminate and not
doing so risks creating security issues in applications.

2

Patches should go to cfe-commits, not cfe-dev. You can also use Phabricator.

3

Could someone please take care of and clean up your contribution documentation?
This is the only project I have heard of where patches are discussed on the commits list (some seriously bad naming), that really should be in BOLD but instead https://libcxx.llvm.org/ only links to the subscription page for cfe-dev, LLVM Developer Policy — LLVM 16.0.0git documentation is rather wishy-washy about it, and the bugzilla doesn't even accept account creation without going via email.
I've seen one-person projects handling it better and not having the gall on top of it to write in the documentation as number 2 goal "Make life as simple and easy for contributors as possible."
I mean seriously, gcc with its copyright assignment hassle is not the reference to shoot for...

4

Could someone please take care of and clean up your contribution documentation

As open source developers, our "currency" is patches and patch review, so "here's a patch to fix some documentation I found confusing" is much more productive than "can someone please fix it"... Patches welcome :wink:

This is the only project I have heard of where patches are discussed on the commits list (some seriously bad naming), that really should be in BOLD but instead https://libcxx.llvm.org/ only links to the subscription page for cfe-dev,

Near the bottom of that page:

"If you want to contribute a patch to libc++, the best place for that is Phabricator. Please include [libc++] in the subject and add cfe-commits as a subscriber."

  LLVM Developer Policy — LLVM 18.0.0git documentation is rather wishy-washy about it,

I suppose that could stand to be slightly more explicit.

  and the bugzilla doesn't even accept account creation without going via email.

That was because we were having a spam problem. Lots of fake accounts, and spammy bug reports.

I've seen one-person projects handling it better and not having the gall on top of it to write in the documentation as number 2 goal "Make life as simple and easy for contributors as possible."
I mean seriously, gcc with its copyright assignment hassle is not the reference to shoot for... >

Patches should go to cfe-commits, not cfe-dev. You can also use Phabricator.

    If the sentinel failed (e.g. due to having reached
    EOF before) or an exception was caught it failed to
    do that.
    While it seems (unfortunately!) not required by the
    specification, libstdc++ does 0-terminate and not
    doing so risks creating 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

A testcase would be good. It should go somewhere in libcxx/test/libcxx, since it will be testing a feature not required by the standard.

Jon