RFR(xs): 8225225: stringStream internal buffer should always be zero terminated

[David Holmes]

Hi Thomas,

On 4/06/2019 6:06 pm, Thomas Stüfe wrote:
> Hi all,
> 
> please review this small fix and the associated test:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8225225
> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8225225-stringstream-should-always-be-zero-terminated/webrev.00/webrev/
> 
> This patch makes sure the internal buffer of stringStream is always zero
> terminated.

I don't see why a newly constructed empty stringstream needs to be 
zero-terminated. We ensure zero termination if we write, and we ensure 
we zero-terminate when we convert via as_string. If someone uses base() 
to directly access our internal buffer (why do we allow that??) then 
they need to be marginally careful when size() is 0.

But if you do make this change then surely here:

  370 char* stringStream::as_string() const {
  371   char* copy = NEW_RESOURCE_ARRAY(char, buffer_pos + 1);
  372   strncpy(copy, buffer, buffer_pos);
  373   copy[buffer_pos] = 0;  // terminating null

we don't need to do this as we must have copied across a terminating 
NUL? (Actually doesn't that apply now, other than when size() == 0?)

Thanks,
David

> It also extends existing gtests to assert this.
> 
> In addition, gtests are expanded to test fixed sized stringStream variants
> too, and to test for buffer overflow for both bufferedStream and
> stringStream (not strictly related to this fix but I was in the area so..).
> 
> Thanks, Thomas
>