RFR: 8197408: Bad pointer comparison and small cleanup in os_linux.cpp

[Robbin Ehn]

Hi David,

On 02/08/2018 01:19 PM, David Holmes wrote:
> On 8/02/2018 10:08 PM, Robbin Ehn wrote:
>> Hi David,
>>
>> On 02/08/2018 12:43 PM, David Holmes wrote:
>>> Hi Robbin,
>>>
>>> On 8/02/2018 7:03 PM, Robbin Ehn wrote:
>>>> Hi all,
>>>>
>>>> There is a bad pointer comparison in os_linux.cpp while looking at that 
>>>
>>> You seem to be missing the fact that OSContainer::cpu_cpuset_memory_nodes() 
>>> can return a pointer or an error code.
>>
>> This is not true for macro:
>> GET_CONTAINER_INFO_CPTR
>>
>> As far I can see?
> 
> Sorry - you're right. I misread current code and misremembered what happened at 
> the initial code review - where I'm sure this "pointer versus error code" issue 
> was also flagged.
> 
>> It returns:
>> if (err != 0)
>>    return (return_type) NULL;
>> or:
>> return os::strdup(mems);
>>
>> If you know a method that returns an integer in a char*, it's broken should be 
>> fixed.
>>
>>>
>>>> I saw some if statement were missing bracket, a lot of extra scopes and 
>>>> complexity in the scoping.
>>>
>>> You'd better check all this with Bob Vandette as its his container support code.
>>
>> Not sure what you mean. It passes container tests.
> 
> In what environment did you run the container tests? Most of the code you've 
> been refactoring deals with various errors and misconfigurations that can occur.
> 
> I'm sure Bob will want a chance to check the refactoring still does as he intended.

On mach5 all platforms with the container test (+ hotspot_tier1) and locally.
I said before, regarding containers, all Linux have cgroups configured so this 
logic always thinks we are in a container and does this logging and calculation.

But no there is no tests for miss-configuration that I found.

Added Bob!

Thanks, Robbin


> 
> Cheers,
> David
> -----
> 
>>
>> Thanks, Robbin
>>
>>>
>>> Cheers,
>>> David
>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~rehn/8197408/webrev/
>>>> Bug:
>>>> http://cr.openjdk.java.net/~rehn/8197408/webrev/
>>>>
>>>> Thanks, Robbin