Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 20 Apr 2015 14:23:58 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Eitan Adler <eadler@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r281758 - head/bin/ed
Message-ID:  <20150420134409.I855@besplex.bde.org>
In-Reply-To: <201504200207.t3K27vFt078041@svn.freebsd.org>
References:  <201504200207.t3K27vFt078041@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 20 Apr 2015, Eitan Adler wrote:

> Log:
>  ed(1): Fix [-Werror=logical-not-parentheses]
>  	/usr/src/bin/ed/glbl.c:64:36: error: logical not is only applied to
>  	theleft hand side of comparison [-Werror=logical-not-parentheses]
>
>  Obtained from:	Dragonfly (1fff89cbaeaa43af720a1f23d9c466b756dd8a58)
>  MFC After:	1 month
> 
> Modified:
>  head/bin/ed/glbl.c
>
> Modified: head/bin/ed/glbl.c
> ==============================================================================
> --- head/bin/ed/glbl.c	Mon Apr 20 00:24:32 2015	(r281757)
> +++ head/bin/ed/glbl.c	Mon Apr 20 02:07:57 2015	(r281758)
> @@ -60,7 +60,7 @@ build_active_list(int isgcmd)
> 			return ERR;
> 		if (isbinary)
> 			NUL_TO_NEWLINE(s, lp->len);
> -		if (!regexec(pat, s, 0, NULL, 0) == isgcmd &&
> +		if (!(regexec(pat, s, 0, NULL, 0) == isgcmd) &&
> 		    set_active_node(lp) < 0)
> 			return ERR;
> 	}

How can this be right?  !(a == b) is an obfuscated way of writing a != b.

The old code seems to have been correct.  regexec() returns a
pseudo-inverted boolean (0 for success, nonzero for error).  isgcmd
is a pure boolean (it is an int function arg, so it could be any
integer, but it is in a function whose only caller passes it the result
of a logical expression).  Presumably it is not inverted.
Pseudo-inverted booleans must be turned into a pure booleans before
being compared with pure booleans, and must usually be inverted.
Presumably, inversion is wanted here.  This was done using the !
operator.

Now, nonsense is done.  There is a type mismatch (integer compared
with boolean), and even if the integer has only boolean values, then
these are probably reversed.

The correct way to turn a pseudo-inverted boolean named `error' into a
non-inverted boolean is (error == 0), not !error, even if `error' is
spelled `regexec(pat, s, 0, NULL, 0)'.  Strict KNF doesn't even allow
using the ! operator for booleans, but ed(1) uses the ! operator a bit.

Sloppy code uses `if (error)' and `if (!error)'.  These work because
they are equivalent to `if (error != 0)' and `if (error == 0')
respectively, and integer 0 represents boolean false uniquely.  Similar
expressions with integer 1 or a boolean variable don't work because
nonzero integers don't represent boolean true uniquely.

A related popular obfuscation is to convert a pseudo-non-inverted boolean
pnib to a non-inverted boolean using !!pnib.  This saves a few characters
and is easy for a quick edit than writing (pnib != 0).

Bruce



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