[10] RFR for 'JDK-8177958: Possible uninitialized char* in vm_version_solaris_sparc.cpp'

[David Holmes]

On 26/04/2017 10:44 PM, Shafi Ahmad wrote:
> Thank you David and Kevin.
>
> Other possible change may be like -
>
> diff -r 762465099d93 src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp
> --- a/src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp	Sat Apr 22 00:21:28 2017 +0000
> +++ b/src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp	Sun Apr 23 23:49:42 2017 -0700
> @@ -404,7 +404,7 @@
>    // is available to us as well
>    Sysinfo cpu_info(SI_CPUBRAND);
>    bool use_solaris_12_api = cpu_info.valid();
> -  const char* impl;
> +  const char* impl = "Unknown";
>    int impl_m = 0;
>    if (use_solaris_12_api) {
>      impl = cpu_info.value();
> @@ -431,7 +431,7 @@
>        kstat_close(kc);
>      }
>    }
> -  assert(impl_m != 0, "Unknown CPU implementation %s", impl);
> +  assert(impl_m != 0, "%s CPU implementation", impl);

I would change to:

assert(impl_m != 0, "Unrecognized CPU implementation: %s", impl);

That assertion is checking the recognition of the impl string, not 
whether we obtained an impl string in the first place.

Thanks,
David

>    features |= impl_m;
>
>    bool is_sun4v = (features & sun4v_m) != 0;
>
> After the above change we will lose the old message format.
>
> Regards,
> Shafi
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Wednesday, April 26, 2017 6:04 PM
>> To: Shafi Ahmad <shafi.s.ahmad at oracle.com>; hotspot-
>> dev at openjdk.java.net
>> Subject: Re: [10] RFR for 'JDK-8177958: Possible uninitialized char* in
>> vm_version_solaris_sparc.cpp'
>>
>> Hi Shafi,
>>
>> On 26/04/2017 6:23 PM, Shafi Ahmad wrote:
>>> Hi,
>>>
>>> Please review the one line trivial  change for the fix of bug 'JDK-8177958:
>> Possible uninitialized char* in vm_version_solaris_sparc.cpp'
>>>
>>> Summary:
>>> I have initialized the uninitialized variable 'imp' with "unknown" as CPU
>> implementation .
>>
>> That fixes the immediate problem - thanks.
>>
>> However the basic problem that kstat failures go unreported/logged persists.
>> If the assert is triggered all we will see is "Unknown CPU implementation
>> unknown" - which is not very enlightening. Perhaps a future enhancement ...
>>
>> David
>>
>>> jdk10 bug: https://bugs.openjdk.java.net/browse/JDK-8177958
>>> webrev link: http://cr.openjdk.java.net/~shshahma/8177958/webrev.00/
>>>
>>> Testing: run jprt
>>>
>>> Regards,
>>> Shafi
>>>