RFR: 8256256: UL should not use heap allocation for output string [v4]

[Yumin Qi]

On Tue, 1 Dec 2020 15:17:02 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix Copyright year
>
> src/hotspot/share/logging/logTagSet.cpp line 128:
> 
>> 126:   assert(ret >= 0, "Log message buffer issue");
>> 127:   size_t newbuf_len = (size_t)ret + prefix_len + 1; // total bytes needed including prefix.
>> 128:   if (newbuf_len <= sizeof(buf)) {
> 
> Hey, I just noted that your patch fixes a subtle bug here if we print with a large prefix `prefix_len > vwrite_buffer_size` but a small message `ret < sizeof(buf)`:
> 
> Before, the code would have printed just the message without the prefix. Now, we correctly go into the malloc path.

Yes, the original code has a bug here.

> src/hotspot/share/logging/logTagSet.cpp line 127:
> 
>> 125:   }
>> 126:   assert(ret >= 0, "Log message buffer issue");
>> 127:   size_t newbuf_len = (size_t)ret + prefix_len + 1; // total bytes needed including prefix.
> 
> After the assert, I would return if ret < 0 to capture the release path.
> Otherwise you could end up malloc'ing a buffer which is too short by one char.

If we return if ret < 0, we should log the buf at least.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1246