Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
On 6 mar 2013, at 00:23, David Holmes wrote:
> On 6/03/2013 9:08 AM, Dean Long wrote:
>> What if allocate_init_map() fails to allocate memory? Shouldn't
>> add_class_share_map_info() follow the pattern of add_map_info():
>> return NULL or "map", and have the caller check for NULL?
>
> AFAICS apart from one seeming bug, if we can't allocate a new map it does no
> harm in terms of the code that looks at the map. The bug is:
>
> 177mp = ph->core->class_share_maps;
> 178if (mp) {
> 179 print_debug("can't locate map_info at 0x%lx, trying class share
> maps\n",
> 180 addr);
>
> where I think 178 should be "if (mp==NULL)".
I think this code is correct, although it took me a while to realize. If it can
find a class share map, it reports that it will start looking at it.
/Staffan
> Everything else will just do nothing upon encountering a NULL map.
>
> The fix addresses the parfait warning and leaves the code functionally
> unchanged. So any robustness issues would need to be done via a follow up RFE.
>
> David
>
>> dl
>>
>> On 3/4/2013 11:39 PM, David Holmes wrote:
>>> Looks fine to me - thanks Staffan!
>>>
>>> David
>>>
>>> On 5/03/2013 5:24 PM, Staffan Larsen wrote:
A very small fix for a warning.
webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/
Thanks,
/Staffan
>>
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
On 3/5/2013 4:48 PM, David Holmes wrote:
On 6/03/2013 10:44 AM, Dean Long wrote:
On 3/5/2013 3:23 PM, David Holmes wrote:
On 6/03/2013 9:08 AM, Dean Long wrote:
What if allocate_init_map() fails to allocate memory? Shouldn't
add_class_share_map_info() follow the pattern of add_map_info():
return NULL or "map", and have the caller check for NULL?
AFAICS apart from one seeming bug, if we can't allocate a new map it
does no harm in terms of the code that looks at the map. The bug is:
177mp = ph->core->class_share_maps;
178if (mp) {
179 print_debug("can't locate map_info at 0x%lx, trying class
share maps\n",
180 addr);
where I think 178 should be "if (mp==NULL)". Everything else will just
do nothing upon encountering a NULL map.
The fix addresses the parfait warning and leaves the code functionally
unchanged. So any robustness issues would need to be done via a follow
up RFE.
Why not just add "return map" at the end of the function? It seems
closer to the intent of the original code, assuming it
addresses the parfait warning.
Nothing expects a return value from that function so what point is
there in returning something? I would expect that to simply produce
another warning/error about a function return value being ignored.
If it complains about that then wouldn't we need to put (void) in front
of all calls to strcat(), for example?
Anyway, I'm OK with the current change if Staffan files an RFE for other
issue.
dl
David
-
dl
David
dl
On 3/4/2013 11:39 PM, David Holmes wrote:
Looks fine to me - thanks Staffan!
David
On 5/03/2013 5:24 PM, Staffan Larsen wrote:
A very small fix for a warning.
webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/
Thanks,
/Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
On 3/5/2013 3:23 PM, David Holmes wrote:
On 6/03/2013 9:08 AM, Dean Long wrote:
What if allocate_init_map() fails to allocate memory? Shouldn't
add_class_share_map_info() follow the pattern of add_map_info():
return NULL or "map", and have the caller check for NULL?
AFAICS apart from one seeming bug, if we can't allocate a new map it
does no harm in terms of the code that looks at the map. The bug is:
177mp = ph->core->class_share_maps;
178if (mp) {
179 print_debug("can't locate map_info at 0x%lx, trying class
share maps\n",
180 addr);
where I think 178 should be "if (mp==NULL)". Everything else will just
do nothing upon encountering a NULL map.
The fix addresses the parfait warning and leaves the code functionally
unchanged. So any robustness issues would need to be done via a follow
up RFE.
Why not just add "return map" at the end of the function? It seems
closer to the intent of the original code, assuming it
addresses the parfait warning.
dl
David
dl
On 3/4/2013 11:39 PM, David Holmes wrote:
Looks fine to me - thanks Staffan!
David
On 5/03/2013 5:24 PM, Staffan Larsen wrote:
A very small fix for a warning.
webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/
Thanks,
/Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
On 6/03/2013 10:44 AM, Dean Long wrote:
On 3/5/2013 3:23 PM, David Holmes wrote:
On 6/03/2013 9:08 AM, Dean Long wrote:
What if allocate_init_map() fails to allocate memory? Shouldn't
add_class_share_map_info() follow the pattern of add_map_info():
return NULL or "map", and have the caller check for NULL?
AFAICS apart from one seeming bug, if we can't allocate a new map it
does no harm in terms of the code that looks at the map. The bug is:
177mp = ph->core->class_share_maps;
178if (mp) {
179 print_debug("can't locate map_info at 0x%lx, trying class
share maps\n",
180 addr);
where I think 178 should be "if (mp==NULL)". Everything else will just
do nothing upon encountering a NULL map.
The fix addresses the parfait warning and leaves the code functionally
unchanged. So any robustness issues would need to be done via a follow
up RFE.
Why not just add "return map" at the end of the function? It seems
closer to the intent of the original code, assuming it
addresses the parfait warning.
Nothing expects a return value from that function so what point is there
in returning something? I would expect that to simply produce another
warning/error about a function return value being ignored.
David
-
dl
David
dl
On 3/4/2013 11:39 PM, David Holmes wrote:
Looks fine to me - thanks Staffan!
David
On 5/03/2013 5:24 PM, Staffan Larsen wrote:
A very small fix for a warning.
webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/
Thanks,
/Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
On 6/03/2013 9:08 AM, Dean Long wrote:
What if allocate_init_map() fails to allocate memory? Shouldn't
add_class_share_map_info() follow the pattern of add_map_info():
return NULL or "map", and have the caller check for NULL?
AFAICS apart from one seeming bug, if we can't allocate a new map it
does no harm in terms of the code that looks at the map. The bug is:
177mp = ph->core->class_share_maps;
178if (mp) {
179 print_debug("can't locate map_info at 0x%lx, trying class
share maps\n",
180 addr);
where I think 178 should be "if (mp==NULL)". Everything else will just
do nothing upon encountering a NULL map.
The fix addresses the parfait warning and leaves the code functionally
unchanged. So any robustness issues would need to be done via a follow
up RFE.
David
dl
On 3/4/2013 11:39 PM, David Holmes wrote:
Looks fine to me - thanks Staffan!
David
On 5/03/2013 5:24 PM, Staffan Larsen wrote:
A very small fix for a warning.
webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/
Thanks,
/Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
What if allocate_init_map() fails to allocate memory? Shouldn't add_class_share_map_info() follow the pattern of add_map_info(): return NULL or "map", and have the caller check for NULL? dl On 3/4/2013 11:39 PM, David Holmes wrote: Looks fine to me - thanks Staffan! David On 5/03/2013 5:24 PM, Staffan Larsen wrote: A very small fix for a warning. webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/ Thanks, /Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
Looks good! Cheers, Mikael On 2013-03-04 23:24, Staffan Larsen wrote: A very small fix for a warning. webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/ Thanks, /Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
Looks good. Thanks, Serguei On 3/4/13 11:24 PM, Staffan Larsen wrote: A very small fix for a warning. webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/ Thanks, /Staffan
Re: RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
Looks fine to me - thanks Staffan! David On 5/03/2013 5:24 PM, Staffan Larsen wrote: A very small fix for a warning. webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/ Thanks, /Staffan
RFR(XS): 8009287 Uninitialised variable in hotspot/agent/src/os/linux/ps_core.c
A very small fix for a warning. webrev: http://cr.openjdk.java.net/~sla/8009287/webrev.00/ Thanks, /Staffan
