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

[Vladimir Kozlov]

Looks like other engineers also think that using // is better. So I am 
withdrawing by objection.

Regards,
Vladimir

On 3/25/14 7:22 AM, Lindenmaier, Goetz wrote:
> 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.
>>>
>>>