svn commit: r270999 - head/sys/kern

[John-Mark Gurney]

Gleb Smirnoff wrote this message on Wed, Sep 03, 2014 at 13:49 +0400:
>   Mateusz, Kostik,
> 
> On Wed, Sep 03, 2014 at 10:55:23AM +0200, Mateusz Guzik wrote:
> M> > Modified: head/sys/kern/kern_proc.c
> M> > ==============================================================================
> M> > --- head/sys/kern/kern_proc.c	Wed Sep  3 08:13:46 2014	(r270998)
> M> > +++ head/sys/kern/kern_proc.c	Wed Sep  3 08:14:07 2014	(r270999)
> M> > @@ -921,10 +921,11 @@ fill_kinfo_proc_only(struct proc *p, str
> M> >  	kp->ki_xstat = p->p_xstat;
> M> >  	kp->ki_acflag = p->p_acflag;
> M> >  	kp->ki_lock = p->p_lock;
> M> > -	if (p->p_pptr)
> M> > +	if (p->p_pptr) {
> M> >  		kp->ki_ppid = proc_realparent(p)->p_pid;
> M> > -	if (p->p_flag & P_TRACED)
> M> > -		kp->ki_tracer = p->p_pptr->p_pid;
> M> > +		if (p->p_flag & P_TRACED)
> M> > +			kp->ki_tracer = p->p_pptr->p_pid;
> M> > +	}
> M> >  }
> M> >  
> M> >  /*
> M> > 
> M> 
> M> p_pptr must be non-NULL if P_TRACED is set. If there is no way to
> M> annotate it for coverity, this change deserves a comment in the code
> M> (and in retrospect previous code should have had appropriate comment as
> M> well).
> 
> Thanks for explanation.
> 
> I'd suggest to leave the change in, since now it is a micro-micro-optimization :)

If you must leave it in, then at least compare the pointer against
NULL, and collapse two if statements into one...

We should never introduce new pointer checks that aren't against NULL...

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."