[8u] RFR 8248924: JDK-8056964 8u backport is not accurate

[Andrew Hughes]

On 08/07/2020 18:48, Zhengyu Gu wrote:
> 
> 
> On 7/8/20 11:37 AM, Andrew Hughes wrote:
>>
>>
>> On 07/07/2020 19:56, Zhengyu Gu wrote:
>>> Please review this small patch that fixes inaccurate backport of
>>> JDK-8056964.
>>>
>>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8248924/webrev.00/
>>>
>>> Test:
>>>    tier1 on Linux 86_64
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>
>> In what way is it not accurate?
> 
> JDK-8056964 original patch uses os::strdup_check_oom() to copy value,
> but the backport converted to strdup().
> 
> There are a couple of differences between the two calls.
> 
> 1) os::strdup_check_oom() memory is tracked by NMT
> 2) when fail to allocate memory, os::strdup_check_oom() results OOM and
> exit VM, while strdup() returns NULL.
> 
> Thanks,
> 
> -Zhengyu
> 

Right. Well, it makes sense that it was changed, given strdup_check_oom
doesn't currently exist in 8u i.e. this depends on the other backport
you posted, JDK-6424123 being
present first.

There are a number of cases of these conversions (we did one for
JDK-8076475, as I mentioned in the other thread, and I referred to ones
for previous fixes in that review). I would consider reverting these
conversions to be part of a backport of 6424123, rather than a separate
fix i.e. such a backport should look at all instances of os::strdup to
see if they need to be changed.

It seems odd to file a bug for a method not being used, when said method
does not currently exist in 8u.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222