Date: Tue, 29 Oct 2013 11:29:05 -0400 From: Mark Johnston <markj@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r257298 - head/lib/libproc Message-ID: <20131029152905.GA2652@charmander.sandvine.com> In-Reply-To: <20131029235717.M905@besplex.bde.org> References: <201310290312.r9T3CVx4064237@svn.freebsd.org> <20131029235717.M905@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 30, 2013 at 12:21:28AM +1100, Bruce Evans wrote:
> On Tue, 29 Oct 2013, Mark Johnston wrote:
>
> > Log:
> > Revert r257248 and fix the problem in a way that doesn't violate style(9).
>
> Why did gcc complain about the original version?
It looks like this is because of the use of -W, which is apparently an
alias for -Wextra; according to the gcc man page, this will trigger a
warning when "an empty body occurs in an if or else statement."
>
> > Modified: head/lib/libproc/_libproc.h
> > ==============================================================================
> > --- head/lib/libproc/_libproc.h Tue Oct 29 02:25:18 2013 (r257297)
> > +++ head/lib/libproc/_libproc.h Tue Oct 29 03:12:31 2013 (r257298)
> > @@ -52,6 +52,6 @@ struct proc_handle {
> > #define DPRINTF(...) warn(__VA_ARGS__)
> > #define DPRINTFX(...) warnx(__VA_ARGS__)
> > #else
> > -#define DPRINTF(...)
> > -#define DPRINTFX(...)
> > +#define DPRINTF(...) do { } while (0)
> > +#define DPRINTFX(...) do { } while (0)
> > #endif
> >
> > Modified: head/lib/libproc/proc_util.c
> > ==============================================================================
> > --- head/lib/libproc/proc_util.c Tue Oct 29 02:25:18 2013 (r257297)
> > +++ head/lib/libproc/proc_util.c Tue Oct 29 03:12:31 2013 (r257298)
> > @@ -145,9 +145,8 @@ proc_wstatus(struct proc_handle *phdl)
> > if (phdl == NULL)
> > return (-1);
> > if (waitpid(phdl->pid, &status, WUNTRACED) < 0) {
> > - if (errno != EINTR) {
> > + if (errno != EINTR)
> > DPRINTF("waitpid");
> > - }
> > return (-1);
> > }
> > if (WIFSTOPPED(status))
>
> Unlike some buggy macros, the null macro expanded to syntactically correct
> code:
>
> if (errno != EINTR)
> ;
>
> so it doesn't need the do-while hack. Empty statements are common in some
> contexts, so compilers shouldn't warn about them. In all versions, the
> macro isn't completely function-like and the if statement is dead code,
> so if compilers warn about too many things then the do-while hack wouldn't
> work here or in most places that it is used.
>
> Nearby style bugs:
> - the error check is for "< 0" instead of for "== -1"
> - the error message is not as loud as old ones. All old ones begin with
> "ERROR: ". The new style is better. "ERROR" is not even redundant.
> It is wrong, since the errors are just warnings.
>
> Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131029152905.GA2652>