pcp
[Top] [All Lists]

Re: [pcp] coding issues and defects uncovered by Coverity scans

To: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Subject: Re: [pcp] coding issues and defects uncovered by Coverity scans
From: Nathan Scott <nathans@xxxxxxxxxx>
Date: Mon, 16 Jan 2012 11:01:11 +1100 (EST)
Cc: Mark Goodwin <mgoodwin@xxxxxxxxxx>, pcp <pcp@xxxxxxxxxxx>
In-reply-to: <1326519152.5145.25.camel@xxxxxxxxxxxxxxxx>
Hi Ken,

----- Original Message -----
> 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.
> 

I don't believe this case happens, going on the man page comments.
The defaults are right for all modern kernels (last 6/7 years, so
I think this issue was really benign anyway).  Thought about simply
removing this, but ended up fixing.  *shrug*.

> 
> 4aa0bb68365670ccf3bf35211b9d6dbbe504f550
> I'm not sure return in mounts_grab_config_info() is correct

*yep* - fixed, thanks.

> a57c46a7b483fbd5ea43234fa8e13226d2e08841
> same concerns for return in process_grab_config_info() as

Fixed.

> e57f80da1619431343b596611cae38d599832cd1
> hmm ... not sure about this one ... proc_stat->cpu_indom is set

This commit fixes a bug introduced in an earlier change to
proc_stat.c via 5550f722f8912e2235e1c396f3a936b237cd1915 -
which effectively undid the work done in linux_init

> 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

I *think* your unmodified code must have been without the
above commit as well?  (else, I'm confused).

> with a breakpoint in refresh_proc_stat() suggests
> proc_stat->cpu_indom is already correctly set up.

I believe the above two commits together (e57f80da and 5550f722)
form a correct unit.

> 7d65a1099198614183f41c8d81b299840c66fdc5
> OK, and not-so-fat bald bastard shuffles off the stage to the
> muffled sounds of public humiliation
> 

Heh.  Don't worry, there was humiliation all round - pretty sure
noone was spared.  :)

cheers.

-- 
Nathan

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [pcp] coding issues and defects uncovered by Coverity scans, Nathan Scott <=