RFR(M): 8038201: Clean up misleading usage of malloc() in init_system_properties_values()

[Lindenmaier, Goetz]

Hi Coleen, 

I also thought about moving that to posix.
But the paths assembled differ a lot on the platforms, 
so I don't think it really helps moving this function:
 - Bsd has two variants, distinguished for __APPLE__.
 - AIX calls LD_LIBRARY_PATH   LIB_PATH and only has one extensions dir, and more simple library path.
 - Solaris has an all different handling of the library path.
Only the first part is similar, except for __APPLE__.  I don't think moving
this part makes the code better maintainable, as it would spread related
code.

Best regards,
  Goetz.

-----Original Message-----
From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
Sent: Dienstag, 25. März 2014 14:46
To: hotspot-dev at openjdk.java.net
Subject: Re: RFR(M): 8038201: Clean up misleading usage of malloc() in init_system_properties_values()


If os::init_system_properties() is really the same for all of these 
platforms, I would like to see it moved to os_posix.cpp.   It doesn't 
seem to make sense to change it the same way 4 times.  Jerry Thornbrough 
is working on the RFE to move "posix" into "xnix", and make more code 
common but this could precede that.

Thanks,
Coleen

On 3/25/14 1:13 AM, Vladimir Kozlov wrote:
> Hi Goetz,
>
> Do you really have to change /**/ comments to //?
> Without that you would get much clear visible changes.
>
> Changes looks fine to me. I wish we could do this in os_posix.cpp but 
> it is for an other time.
>
> Thanks,
> Vladimir
>
> On 3/24/14 2:50 PM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> please review and test this change. I please need a sponsor.
>> http://cr.openjdk.java.net/~goetz/webrevs/8038201-sec/webrev.00/
>>
>> This change addresses the implementation of 
>> init_system_properties_values
>> on aix, linux, solaris and bsd.
>> In init_system_properties_values a macro was defined mapping malloc to
>> NEW_C_HEAP_ARRAY. NEW_C_HEAP_ARRAY checks for successful allocation
>> or exits the VM.  The code of this method handles the allocated 
>> pointers as
>> allocated with real malloc, i.e.,  there were checks for NULL and 
>> calls to free().
>>
>> This change replaces the macro malloc with NEW_C_HEAP_ARRAY and removes
>> the unnecessary checks making the code clearer.  Also, it uses a 
>> local array where
>> possible.
>>
>> The allocated memory is passed to calls that end up at 
>> SystemProperty::set_value().
>> set_value copies the strings passed.  Thus the memory allocated in
>> init_system_properties_values must be freed after these calls. Most 
>> of these
>> frees were missing.
>>
>> This change adds the missing frees.
>>
>> Testing this change I ran into a warning in ppc code which I fixed, too.
>> I did some local test.  I'll get broader tests by Wednesday.
>>
>> Best regards,
>>    Goetz.
>>
>>