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

Coleen Phillimore coleen.phillimore at oracle.com
Tue Mar 25 14:20:27 UTC 2014

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

Okay, I saw your reply to Vladimir after I made this comment.  They just 
looked the same so it's always a question.   Too bad.

I didn't actually look at the contents of the changes.  If you need 
another reviewer, I will, but I think you have good reviewers in 
Vladimir and Dmitry.


> 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.

More information about the hotspot-dev mailing list