Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Sep 2014 12:10:28 -0700
From:      John-Mark Gurney <jmg@funkthat.com>
To:        Gleb Smirnoff <glebius@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, Mateusz Guzik <mjguzik@gmail.com>, kib@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r270999 - head/sys/kern
Message-ID:  <20140903191028.GG71691@funkthat.com>
In-Reply-To: <20140903094916.GO7693@FreeBSD.org>
References:  <201409030814.s838E7A2084257@svn.freebsd.org> <20140903085523.GB13871@dft-labs.eu> <20140903094916.GO7693@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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."



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140903191028.GG71691>