On Thu, Nov 09, 2017 at 05:37:32PM -0800, Volodymyr Sapsai wrote:
> On Nov 9, 2017, at 12:13, Reimar Döffinger <reimar.doeffin...@gmx.de> wrote:
> >
> > 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.
> I think it makes sense to cover wchar_t as well. And I think it would be
> useful to test the case with specified delimiter too. Most likely
> implementation should be reused for the case with explicit delimiter and
> default new line delimiter. But I prefer not to rely on this assumption and
> test explicitly.
Well, it was rather trivial to do anyway, so done.
> > More tricky would be to add a test for the _LIBCPP_NO_EXCEPTIONS case,
> > is there any code testing that at all?
> According to tests, exceptions are tested in various ways. Let me find how to
> configure the build to run tests in these modes.
There aren't many references to that mode, and I am not sure
running in a separate mode would make for a good test anyway.
Maybe not how it should done, but for now I added a separate file
that just defines _LIBCPP_NO_EXCEPTIONS before the includes.
>From abfb27562be9bf2838c3357403d1d23297e63f8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <reimar.doeffin...@gmx.de>
Date: Thu, 7 Sep 2017 08:42:10 +0200
Subject: [PATCH] Ensure std::istream::getline always 0-terminates string.
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.
Note that means behaviour is inconsistent with the
std::getline version returning a std::string, which
does not modify the string in case of error, but
that is both less serious and matches behaviour
of e.g. libstdc++, so leave it as-is for now.
---
include/istream | 6 ++-
.../getline_pointer_size.pass.cpp | 14 ++++++
.../getline_pointer_size_chart.pass.cpp | 14 ++++++
.../getline_pointer_size_exception.pass.cpp | 52 ++++++++++++++++++++++
4 files changed, 84 insertions(+), 2 deletions(-)
create mode 100644 test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_exception.pass.cpp
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
diff --git a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
index 465824a65..d0da78b5c 100644
--- a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
+++ b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
@@ -59,6 +59,13 @@ int main()
assert(!is.fail());
assert(std::string(s) == " ");
assert(is.gcount() == 1);
+ is.getline(s, 5);
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert( is.eof());
+ assert( is.fail());
+ assert(std::string(s) == "");
+ assert(is.gcount() == 0);
}
{
testbuf<wchar_t> sb(L" \n \n ");
@@ -79,5 +86,12 @@ int main()
assert(!is.fail());
assert(std::wstring(s) == L" ");
assert(is.gcount() == 1);
+ is.getline(s, 5);
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert( is.eof());
+ assert( is.fail());
+ assert(std::wstring(s) == L"");
+ assert(is.gcount() == 0);
}
}
diff --git a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp
index 736295996..084104110 100644
--- a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp
+++ b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp
@@ -59,6 +59,13 @@ int main()
assert(!is.fail());
assert(std::string(s) == " ");
assert(is.gcount() == 1);
+ is.getline(s, 5, '*');
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert( is.eof());
+ assert( is.fail());
+ assert(std::string(s) == "");
+ assert(is.gcount() == 0);
}
{
testbuf<wchar_t> sb(L" * * ");
@@ -79,5 +86,12 @@ int main()
assert(!is.fail());
assert(std::wstring(s) == L" ");
assert(is.gcount() == 1);
+ is.getline(s, 5, L'*');
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert( is.eof());
+ assert( is.fail());
+ assert(std::wstring(s) == L"");
+ assert(is.gcount() == 0);
}
}
diff --git a/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_exception.pass.cpp b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_exception.pass.cpp
new file mode 100644
index 000000000..b319ccd86
--- /dev/null
+++ b/test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_exception.pass.cpp
@@ -0,0 +1,52 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <istream>
+
+// basic_istream<charT,traits>& getline(char_type* s, streamsize n);
+
+#define _LIBCPP_NO_EXCEPTIONS 1
+#include <stdexcept>
+#include <istream>
+#include <cassert>
+
+template <class CharT>
+struct throwing_testbuf
+ : public std::basic_streambuf<CharT>
+{
+ virtual typename std::basic_streambuf<CharT>::int_type underflow() { throw std::runtime_error(""); }
+};
+
+int main()
+{
+ {
+ throwing_testbuf<char> sb;
+ std::istream is(&sb);
+ char s[5];
+ is.getline(s, 5);
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert(!is.eof());
+ assert( is.fail());
+ assert(std::string(s) == "");
+ assert(is.gcount() == 0);
+ }
+ {
+ throwing_testbuf<wchar_t> sb;
+ std::wistream is(&sb);
+ wchar_t s[5];
+ is.getline(s, 5);
+ // check that even in error case the buffer
+ // is properly 0-terminated
+ assert(!is.eof());
+ assert( is.fail());
+ assert(std::wstring(s) == L"");
+ assert(is.gcount() == 0);
+ }
+}
--
2.15.0
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
