RFR(trivial): 8231024: Improve the debug info when the output is truncated

[Jie Fu]

Hi Ioi and David,

Thank you very much for your review and valuable comments.

Updated: http://cr.openjdk.java.net/~jiefu/8231024/webrev.02/
  -1) Change the error message which is somewhat misleading
  -2) Rename "written" to "required_len"

- Before the patch
------------------------------------
[2019-09-14 16:07:55,752] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
warning: increase O_BUFLEN in ostream.hpp -- output truncated
[2019-09-14 16:07:55,758] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
warning: increase O_BUFLEN in ostream.hpp -- output truncated
[2019-09-14 16:07:55,763] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
warning: increase O_BUFLEN in ostream.hpp -- output truncated
...
------------------------------------

- After the patch
------------------------------------
[2019-09-17 09:49:21,567] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
warning: outputStream::do_vsnprintf output truncated -- buffer length is 
2000 bytes but 2013 bytes are needed.
[2019-09-17 09:49:21,571] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
warning: outputStream::do_vsnprintf output truncated -- buffer length is 
2000 bytes but 2031 bytes are needed.
[2019-09-17 09:49:21,576] Agent[1]: stderr: OpenJDK 64-Bit Server VM 
warning: outputStream::do_vsnprintf output truncated -- buffer length is 
2000 bytes but 2049 bytes are needed.
...
------------------------------------

Could you please review it and give me some advice?

Thanks a lot.
Best regards,
Jie

On 2019/9/17 上午7:20, Ioi Lam wrote:
>
>
> On 9/16/19 12:44 AM, David Holmes wrote:
>> On 16/09/2019 5:09 pm, Jie Fu wrote:
>>> Hi David,
>>>
>>> Sorry for the confusion.
>>
>> The confusion is mine, not from you :)
>>
>>> On 2019/9/16 下午2:40, David Holmes wrote:
>>>> Okay I'm confused. I had assumed that the passed in buffer size 
>>>> must be different in each case because of the 2014/2032/2050 values 
>>>> in the message, but in fact they are all sized at 2000. 
>>> According to the source code, the buffer size can be either 
>>> 2000(O_BUFLEN) [1] or 2048(BUFLEN) [2].
>>> So it isn't always 2000.
>>
>> It is 2000 is all three examples of the output that you gave.
>
> xmlStream::va_tag() uses 2*K.
>
>>
>>>> That makes no sense
>>> I think it makes sense.
>>> It is still unclear which one (O_BUFLEN or BUFLEN?) should be 
>>> increased if the current buffer size wasn't printed.
>>
>> You need to know the original caller that passed in the buffer size.
>>
>> But we can't just keep increasing the buffer size. We probably need 
>> to examine the callers to see what amount of information is trying to 
>> be passed through. Then decide whether to adjust what the callers are 
>> passing through, or adjust the buffer size - perhaps on a callsite by 
>> callsite basis rather than just bumping O_BUFLEN or BUFLEN.
>>
>
> I think the message should stop saying "increase O_BUFLEN" -- probably 
> the reviewers will reject all future increase of O_BUFLEN and will 
> suggest fixing the caller of outputStream::do_vsnprintf instead.
>
> Callers of outputStream::do_vsnprintf don't necessarily use O_BUFLEN. 
> E.g., xmlStream::va_tag allocates its own buffer. 
> VMError::report_and_die() also uses its own buffer -- it uses O_BUFLEN 
> today, but there's no guarantee or requirement that it always does that.
>
> How about saying something like "outputStream::do_vsnprintf output 
> truncated -- buffer length is 2000 bytes but 2100 bytes are needed."
>
>
>>>> - how can we have written e.g. 2014 bytes to a buffer of only 2000 
>>>> bytes? 
>>>
>>> The output is truncated since the current buffer size is too small 
>>> (e.g., 2000 < 2014).
>>
>> Ah sorry - I'd missed that aspect of vsnprintf. We're calling:
>>
>>  102     int written = os::vsnprintf(buffer, buflen, format, ap);
>>
>> but it isn't "written" when it exceeds buflen, it's the required 
>> buffer size.
>>
>
> I think the variable "written" should be renamed to "required".
>
> Thanks
> - Ioi
>
>> David
>> -----
>>
>>>
>>>> Do we have a bug in os::vsnprintf?
>>>
>>> I don't think so.
>>>
>>> Thanks a lot.
>>> Best regards,
>>> Jie
>>>
>>> [1] 
>>> http://hg.openjdk.java.net/jdk/jdk/file/24df796eef3d/src/hotspot/share/utilities/ostream.cpp#l124 
>>>
>>> [2] 
>>> http://hg.openjdk.java.net/jdk/jdk/file/24df796eef3d/src/hotspot/share/utilities/xmlstream.cpp#l138 
>>>
>>>
>>>
>