RFR(XXS): 7133111: libsaproc debug print should be printed as unsigned long to fit large numbers on 64bit platform

[serguei.spitsyn at oracle.com]

Looks good.

Thanks,
Serguei

On 4/23/12 2:05 AM, Staffan Larsen wrote:
> Can I get a Review for this tiny change, please.
>
> Thanks,
> /Staffan
>
> On 18 apr 2012, at 13:54, Rickard Bäckman wrote:
>
>> Staffan,
>>
>> I think the change looks good.
>>
>> /R
>>
>> On Apr 17, 2012, at 11:38 AM, Staffan Larsen wrote:
>>
>>> David,
>>>
>>> You are right, thanks for catching this. The correct format specifier is %zu for a size_t. I've created a new bug 7162063 for this change since I had already submitted the incorrect change.
>>>
>>> Please review the diff below.
>>>
>>> Thanks,
>>> /Staffan
>>>
>>>
>>> --- a/agent/src/os/linux/ps_core.c
>>> +++ b/agent/src/os/linux/ps_core.c
>>> @@ -440,7 +440,7 @@
>>>       int j = 0;
>>>       print_debug("---- sorted virtual address map ----\n");
>>>       for (j = 0; j<  ph->core->num_maps; j++) {
>>> -        print_debug("base = 0x%lx\tsize = %zd\n", ph->core->map_array[j]->vaddr,
>>> +        print_debug("base = 0x%lx\tsize = %zu\n", ph->core->map_array[j]->vaddr,
>>>                                          ph->core->map_array[j]->memsz);
>>>       }
>>>    }
>>>
>>> On 16 apr 2012, at 07:08, David Holmes wrote:
>>>
>>>> On 5/04/2012 10:25 PM, Staffan Larsen wrote:
>>>>> Please review the following one-character fix to a printf format string. A 'z' is added to the printout of a size_t field.
>>>> Sorry I'm late to the party and this code already shipped. The z length modifier is not linux specific but was added as part of the C99 standard. Is it also a gcc extension enabled by default (I don't think we run in C99 mode by default) ?
>>>>
>>>> But z simply changes the following d/i/o/u/x/X to indicate it refers to a size_t - which is somewhat confusing as size_t is unsigned, so does %zd print a signed or unsigned representation? If signed then the bug still exists for really large numbers.
>>>>
>>>> Note the same bug exists in the BSD version of the code.
>>>>
>>>> I agree with Dan that the reference to "unsigned long" in the synopsis is very confusing - please change the synopsis to reflect the actual problem e.g: "debug print should format size_t correctly".
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> /Staffan
>>>>>
>>>>>
>>>>> diff --git a/agent/src/os/linux/ps_core.c b/agent/src/os/linux/ps_core.c
>>>>> --- a/agent/src/os/linux/ps_core.c
>>>>> +++ b/agent/src/os/linux/ps_core.c
>>>>> @@ -440,7 +440,7 @@
>>>>>       int j = 0;
>>>>>       print_debug("---- sorted virtual address map ----\n");
>>>>>       for (j = 0; j<   ph->core->num_maps; j++) {
>>>>> -        print_debug("base = 0x%lx\tsize = %d\n", ph->core->map_array[j]->vaddr,
>>>>> +        print_debug("base = 0x%lx\tsize = %zd\n", ph->core->map_array[j]->vaddr,
>>>>>                                          ph->core->map_array[j]->memsz);
>>>>>       }
>>>>>    }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120423/17f7ce47/attachment.html