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

[Volodymyr Sapsai via cfe-commits]

On Nov 20, 2017, at 16:33, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> 
> On 20 November 2017 22:19:04 CET, Volodymyr Sapsai <vsapsai at apple.com <mailto:vsapsai at apple.com>> wrote:
>> On Nov 20, 2017, at 11:32, Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> wrote:
>>> 
>>> On Mon, Nov 20, 2017 at 11:02:13AM -0800, Volodymyr Sapsai wrote:
>>>>>    catch (...)
>>>>>    {
>>>>> +        if (__n > 0)
>>>>> +            *__s = char_type();
>>>>>        this->__set_badbit_and_consider_rethrow();
>>>>>    }
>>>> 
>>>> or maybe something else?
>>> 
>>> That one (note that the __set_badbit_and_consider_rethrow
>>> will never re-throw in this case).
>> 
>> But by #define _LIBCPP_NO_EXCEPTIONS 1 you exclude this block at
>> preprocessing step
>> 
>>> #ifndef _LIBCPP_NO_EXCEPTIONS
>>>     }
>>>     catch (...)
>>>     {
>>> +        if (__n > 0)
>>> +            *__s = char_type();
>>>         this->__set_badbit_and_consider_rethrow();
>>>     }
>>> #endif  // _LIBCPP_NO_EXCEPTIONS
>> 
>> And looks like getline_pointer_size_exception.pass.cpp doesn’t execute
>> this code.
> 
> Yes, that was complete nonsense, I somehow always read #ifdef where there was an #ifndef...
> Not sure then if that 0 termination should be there (and be tested) or better to not have it in the exception case at all (I haven't checked if the exception is always re-thrown or not, which might be relevant).
> I am a bit unclear about that whole code there that catches exceptions and sets the bad bit and how it maps to the specification.

Standard mentions

> Otherwise, if the sentry constructor exits by throwing an exception or if the sentry object returns false, when converted to a value of type bool, the function returns without attempting to obtain any input. In either case the number of extracted characters is set to 0; unformatted input functions taking a character array of non-zero size as an argument shall also store a null character (using charT()) in the first location of the array.

My understanding is that if sentry constructor throws an exception we need to null-terminate a character array. Null-termination isn’t specified explicitly for exceptions thrown in other cases but I think it would be more cumbersome to perform null-termination only for sentry exceptions and not for others. Regarding the safety, we control __s, don’t pass it anywhere, it never points beyond __n - I think null-termination should be safe. Also libstdc++ does null-termination when exceptions are thrown.

As for rethrowing exception, looks like exceptions flag std::istream::eofbit causes exception to be thrown and std::istream::badbit to be rethrown according to
> If (exceptions()&badbit) != 0 then the exception is rethrown.
And looks like libstdc++ rethrows exception even if badbit is not set.

If you feel comfortable, I can finish exception tests myself and commit the patch. How does it sound?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171120/8ce791a8/attachment-0001.html>