On Fri, 31 Oct 2025 at 18:00 +0000, Jonathan Wakely wrote:
On Mon, 27 Oct 2025 at 22:46 -0400, Patrick Palka wrote:
From: Jonathan Wakely <[email protected]>
Tested on x86_64-pc-linux-gnu. Patch originally written by Jonathan and
completed by me (mainly testsuite adjustments).
Thanks for taking on the task of figuring out what needing changing in
the tests. IIRC I made the changes to stringbuf and when tests failed
in unexpected ways I got confused and gave up.
I don't understand the stringbuf API enough to determine what the
correct value of egptr() - eback() (i.e. the get area size) should be in
the 26250.cc tests aftering calling overflow(). It's currently zero
after this patch, which I'm pretty sure is wrong?
[stringbuf.virtuals]/8 says: If ios_base::in is set in mode, the
function alters the read end pointer egptr() to point just past the new
write position.
In light of that I guess egptr() - eback() should still be 1 after this
patch then? That'd require making stringbuf::overflow() more
consistently set egptr() I believe.
As written, I think the "alters the read end pointer egptr()" wording
only applies in the case where overflow needs to make a new write
position available, i.e. needs to reallocate.
When we've been initialized with the SSO buffer the initial capacity
means that the single call to overflow in 26250.cc doesn't need to
reallocate. So I think it's correct that after your change, the size
of the get area isn't change by the overflow('x') call. Because it
doesn't reallocate.
But then what about for:
buf.sputn("01234567890abcdef", 16); // Exceed SSO buffer
buf.overflow('x');
VERIFY( buf.egptr() - buf.eback() == ? );
I would think the get area size should be 17? But other implementations
including us before this patch say 16, so I'm pretty confused.
Again, I think this depends on whether overflow('x') reallocates. If
the capacity after sputn means a write position is available, then my
reading of the spec says we don't change egptr().
But that doesn't really make sense to me. Surely the point of moving
the egptr is that a character has been written to the put area and
that character should now be available for reading, so the get area
should expand to include the just-written character. But that should
happen any time a character is written to the put area, not only when
reallocation was needed?
Ah, but the newly-written character is made available, because the
high_mark is moved when writing a new character. The egptr() only
needs to be adjusted when reallocating.
I think what we're doing is correct, and your changes to the tests are
the right ones to adjust to the new expected behaviour.
It might be good if the 26250.cc tests were adjusted to still test
that egptr is moved after reallocating, since that's what it is
intended to test. It's just that with your changes, the test no longer
reallocates.
So something like this (untested!)
#if _GLIBCXX_USE_CXX11_ABI
// No reallocation happened on overflow above.
VERIFY( buf.egptr() - buf.eback() == 0 );
// Fill the write area and then overflow again:
std::string fill(write_positions, 'x');
buf.sputn(fill.c_str(), fill.size());
std::ptrdiff_t get_size = buf.egptr() - buf.eback();
buf.overflow('x');
VERIFY( buf.egptr() - buf.eback() == get_size + 1 );
#else
VERIFY( buf.egptr() - buf.eback() == 1 );
#endif
This assumes the following change earlier in the test:
// not required but good for efficiency
// NB: we are implementing DR 169 and DR 432
- const int write_positions = buf.epptr() - buf.pbase();
+ const int write_positions = buf.epptr() - buf.pptr();
VERIFY( write_positions > 1 );
// 27.7.1.3, p8:
Because surely the number of available write positions is the number
of positions after pptr(), not the number of positions after pbase()?
