Looks good to me too.
David
On 23/04/2012 7:05 PM, 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);
}
}
