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

[Markus Grönlund]

Looks good!

/Markus

> -----Original Message-----
> From: Staffan Larsen
> Sent: den 23 april 2012 11:05
> To: serviceability-dev serviceability-dev at openjdk.java.net
> Subject: Re: RFR(XXS): 7133111: libsaproc debug print should be printed
> as unsigned long to fit large numbers on 64bit platform
> 
> 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);
> >>>>      }
> >>>>   }
> >>
> >
>