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

[Lindenmaier, Goetz]

Hi Vladimir, 

before I got Dmitrys Mail I already had reverted the comment changes:
http://cr.openjdk.java.net/~goetz/webrevs/8038201-sec/webrev.01/

I don't really like this better.  Code now jumps between the
different comment styles all over, see e.g. solaris.
And the lines are touched anyways due to the indentation fixes.
I assume hg annotate will mark that all as edited by this change.
Or does it also ignore whitespace changes?
I would put the looks of the code over the looks of the patch.
But in the end I don't care.

Best regards,
  Goetz.



-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Dienstag, 25. März 2014 09:02
To: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
Subject: Re: RFR(M): 8038201: Clean up misleading usage of malloc() in init_system_properties_values()

On 3/25/14 12:57 AM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> I thought I have to clean up formatting of the function if I touch it.
> Should I revert this?  Should I also go back to indenting by four?

I don't think we require only // comments.

Fixing indention and adding {} is good.

>
> I don't think this is a good candidate for posix, as the major
> task is to assemble the paths, and they differ a lot between
> the four oses.  Only setting java_home and dll_dir are similar.

Okay.

Thanks,
Vladimir

>
> Best regards,
>    Goetz.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Dienstag, 25. März 2014 06:14
> To: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> Subject: Re: RFR(M): 8038201: Clean up misleading usage of malloc() in init_system_properties_values()
>
> 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.
>>
>>