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);
>>        }
>>     }

Reply via email to