[libcxx-commits] [PATCH] D146294: [libcxx] Fix crash in std::stringstream with payload >= INT_MAX

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

I have no idea how the stream stuff is implemented, so I can't say whether the implementation changes look good. I've got a few nits in the test.

In D146294#4206359 <https://reviews.llvm.org/D146294#4206359>, @azat wrote:

> Test failures looks unrelated, can someone take a look at the patch, @ldionne you can take a look ?
> Thanks in advance!

Yes, the failures look unrelated.



================
Comment at: libcxx/test/std/input.output/string.streams/stringstream.members/gcount.pass.cpp:18
+
+#include "test_macros.h"
+
----------------
This seems unused.


================
Comment at: libcxx/test/std/input.output/string.streams/stringstream.members/gcount.pass.cpp:11-12
+
+// Test that tellp() does not breaks the stringstream after INT_MAX, due to use
+// of pbump() that accept int.
+
----------------



================
Comment at: libcxx/test/std/input.output/string.streams/stringstream.members/gcount.pass.cpp:24
+
+  for (size_t i = 0; i < (2ULL << 30) - payload.size(); i += payload.size()) {
+    assert(ss.tellp() != -1);
----------------
azat wrote:
> I'm not sure that the test that allocates 2GiB of RAM is good for your CI, so please let me know.
This should be fine. It's probably not actually allocated anyways.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146294/new/

https://reviews.llvm.org/D146294