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

[Volodymyr Sapsai via cfe-commits]

On Nov 19, 2017, at 08:07, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> 
> On Wed, Nov 15, 2017 at 11:35:56AM -0800, Volodymyr Sapsai wrote:
>> On Nov 12, 2017, at 12:37, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
>> libc++ can be built with exceptions enabled or disabled (see LIBCXX_ENABLE_EXCEPTIONS <http://libcxx.llvm.org/docs/BuildingLibcxx.html#cmdoption-arg-libcxx-enable-exceptions>) and we need to support both configurations. The problem with your _LIBCPP_NO_EXCEPTIONS approach is that it won't work when exceptions are disabled. Various exception-related tests are usually guarded with TEST_HAS_NO_EXCEPTIONS macro defined in “test_macros.h”. You can check other tests for examples of TEST_HAS_NO_EXCEPTIONS usage, I had in mind something like
>> 
>> #ifndef TEST_HAS_NO_EXCEPTIONS
>>    {
>>        testbuf<char> sb(" ");
>>        std::istream is(&sb);
>>        char s[5] = "test";
>>        is.exceptions(std::istream::eofbit | std::istream::badbit);
>>        try
>>        {
>>            is.getline(s, 5);
>>            assert(false);
> 
> That doesn't make sense to me, the whole point is to test the
> code-path in getline that swallows the exception.
> So if you meant for that assert to be there, you're
> not trying to test what I am trying to test.
> Note that the intent is not to ensure 0-termination
> when getline actually exits via an exception, I doubt
> that would be safe to do really (except maybe at the start
> of the loop before the code that might trigger an exception,
> but that would cost performance) and anyway code
> handling exceptions can be expected to not make assumptions
> that the result is in a "safe" state.
> The problem is when the code just swallows the exception
> (when exceptions are disabled, presumably for interoperability
> with code compiled with exceptions enabled? There is no documentation
> why getline catches exceptions when they are disabled),
> then I think the only reasonable choice is to 0-terminate,
> even if it is risky.
> Looking at the other tests it seems that generally functionality
> simply isn't tested at all when exceptions are disabled
> (e.g. stoul.pass.cpp), which seems unfortunate.
> That level of testing is easy to achieve here as well,
> just leave out the test file I added.
> If you want it though, I updated it to not fail (though
> not actually test anything either) if exceptions are
> disabled, and added a comment on what a huge hack it is
> (as the code-path for disabled exceptions is tested if and
> only if exceptions are actually enabled).
> <0001-Ensure-std-istream-getline-always-0-terminates-strin.patch>

I got confused here. When you mention "code path for disabled exceptions”, what exactly code do you have in mind? Is that

> -            if (__n > 0)
> -                *__s = char_type();
>              if (__gc_ == 0)
>                 __err |= ios_base::failbit;
>              this->setstate(__err);
>          }
> +        if (__n > 0)
> +            *__s = char_type();

or

>      catch (...)
>      {
> +        if (__n > 0)
> +            *__s = char_type();
>          this->__set_badbit_and_consider_rethrow();
>      }

or maybe something else?