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

[Reimar Döffinger via cfe-commits]

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).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Ensure-std-istream-getline-always-0-terminates-strin.patch
Type: text/x-diff
Size: 7239 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171119/f636a53a/attachment.patch>