On Fri, 2012-01-13 at 12:58 +1100, Nathan Scott wrote:
> Attached is a text version of the html, with FIXED/IGNORED annotations
> in the final column for the ones I've looked at.
>
> I've found one subtle case where fixing something broke something (see
> commit e57f80da1619431343b596611cae38d599832cd1), so I'd appreciate an
> eye being cast over all these changes too - sometimes its immediately
> obvious what the best fix approach is, so any feedback would be good.
>
> cheers.
>
In reverse chronological order ...
d8f0d28e4bfa4505d92ed5307ed4632205e3a6b7
Are there cases where "point" is not present, e.g. 2.3 not 2.3.6?
If so, need to snarf the result from sscanf() and make the first
block testing only major & minor guarded by sscanf() >= 3, not
== 3, and then test for sscanf() == 3 in the later block where
point is tested.
6ca653424dd265dd094f356b02d382bcf0a822fd
OK
d1552a02e6828d5a5248b01343f33f1d6936b5dd
OK ... not only is this never used, but loop.c is expunged in PCP 4.0
4aa0bb68365670ccf3bf35211b9d6dbbe504f550
I'm not sure return in mounts_grab_config_info() is correct
here ... need to clean up the instance domain, so something
like
if (mounts != NULL) {
free(mounts);
mounts = NULL;
mount_number = 0;
}
goto done;
and at the end of the while loop, move the fclose(fp) up and add
the branch label
}
fclose(fp);
done:
if (mounts == NULL) {
... etc
64fa9ceb3bee1fb44578f2ee5c583b8839228ff5
OK
a57c46a7b483fbd5ea43234fa8e13226d2e08841
same concerns for return in process_grab_config_info() as
outlined for mounts_grab_config_info() above ... the code seems
to be a close copy.
e57f80da1619431343b596611cae38d599832cd1
hmm ... not sure about this one ... proc_stat->cpu_indom is set
to &indomtab[CPU_INDOM] in linux_init(). indomtab[] cannot be
relocated, and refresh_proc_stat() is only called from
linux_refresh() where the proc_stat argument is the address of
the same is a global static proc_stat that is initializaed in
linux_init(). And this gdb snippet from the unmodified code
with a breakpoint in refresh_proc_stat() suggests
proc_stat->cpu_indom is already correctly set up.
(gdb) p proc_stat
$1 = (proc_stat_t *) 0x7f16957c5ae0
(gdb) p *proc_stat
$2 = {user = 254930, sys = 61969, nice = 3504, idle = 36760546, wait =
33708,
irq = 27, sirq = 804, steal = 0, guest = 31940, ncpu = 6,
p_user = 0x11a3d00, p_sys = 0x11a3d80, p_nice = 0x11a3d40,
p_idle = 0x11a3dc0, p_wait = 0x11a3f20, p_irq = 0x11a3f60,
p_sirq = 0x11a3fa0, p_steal = 0x11a3fe0, p_guest = 0x11a4020,
n_user = 0x11a3e00, n_sys = 0x11a4080, n_nice = 0x11a4060,
n_idle = 0x11a40a0, n_wait = 0x11a40c0, n_irq = 0x11a40e0,
n_sirq = 0x11a4100, n_steal = 0x11a4120, n_guest = 0x11a4140, ndisk =
0,
page = {0, 0}, swap = {0, 0}, intr = 44274966, ctxt = 35822439,
btime = 1326454366, processes = 13289, cpu_indom = 0x7f16957bdd40, hz
= 100}
(gdb) p *proc_stat->cpu_indom
$3 = {it_indom = 251658240, it_numinst = 6, it_set = 0x11a1e40}
(gdb) p indomtab[CPU_INDOM]
$4 = {it_indom = 251658240, it_numinst = 6, it_set = 0x11a1e40}
(gdb) p &indomtab[CPU_INDOM]
$5 = (pmdaIndom *) 0x7f16957bdd40
4ca7322b29388290a08fa9291e2837c8015f800f
OK
1d0008ddea554e83dc588f9318de1e1016146958
OK
67064c5a97d44ea8aa8207dc58251d3ca353e367
OK and embarrassing as this is clearly my code!
700a6a0187de8629c2ddb3617145a9261ae7eb25
OK
7d65a1099198614183f41c8d81b299840c66fdc5
OK, and not-so-fat bald bastard shuffles off the stage to the
muffled sounds of public humiliation
5550f722f8912e2235e1c396f3a936b237cd1915
OK
All in all, a good day's fishing, with some real clangers fixed, but it
does highlight the need for care and reviewing changes in code that was
not known to be broken before the coverity analysis.
Thanks, Nathan.
|