From owner-svn-src-head@FreeBSD.ORG Mon Apr 20 20:11:35 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9826BD52; Mon, 20 Apr 2015 20:11:35 +0000 (UTC) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id C6101BFC; Mon, 20 Apr 2015 20:11:34 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id E726F424073; Tue, 21 Apr 2015 05:53:10 +1000 (AEST) Date: Tue, 21 Apr 2015 05:53:09 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eitan Adler cc: Bruce Evans , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Subject: Re: svn commit: r281758 - head/bin/ed In-Reply-To: Message-ID: <20150421050859.H11701@besplex.bde.org> References: <201504200207.t3K27vFt078041@svn.freebsd.org> <20150420134409.I855@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=L/MkHYj8 c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=TEllcOey7MfUqoLxoeYA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Apr 2015 20:11:35 -0000 On Mon, 20 Apr 2015, Eitan Adler wrote: > On 19 April 2015 at 21:23, Bruce Evans wrote: >> On Mon, 20 Apr 2015, Eitan Adler wrote: >>> ... >>> --- 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. > bah! > How does something like the following look? Sorry, not good. I don't like stdbool. Here using it just enlarges and unportablize the code. > Index: ed.h > =================================================================== > --- ed.h (revision 281759) > +++ ed.h (working copy) > @@ -33,6 +33,7 @@ > #include > #include > #include > +#include > #include > #include > #include ed already has bad style, with *.h included in ed.h... > @@ -191,7 +192,7 @@ int put_des_char(int, FILE *); > void add_line_node(line_t *); > int append_lines(long); > int apply_subst_template(const char *, regmatch_t *, int, int); > -int build_active_list(int); > +int build_active_list(bool); It would be impossible to even declare a function that uses bool in a header without increasing the pollution. Actually, this is easy in C99 and only difficult to do portably. _Bool is standard in C99, so the include is only needed for portability to systems that have but not _Bool, or if you want to spell _Bool as bool in C99. The header (and program) otherwise doesn't use many fancy types. > int cbc_decode(unsigned char *, FILE *); > int cbc_encode(unsigned char *, int, FILE *); > int check_addr_range(long, long); It does use FILE. That requires the pollution. > Index: glbl.c > =================================================================== > --- glbl.c (revision 281759) > +++ glbl.c (working copy) > @@ -60,7 +60,7 @@ int > 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; > } This still has the obfuscation of using ! on a non-boolean, and then needs excessive parentheses to defeat the compiler warning about (a side effect of) this. Parentheses are needed for syntactical reasons in the non-obfuscated version: if ((regexec(pat, s, 0, NULL, 0) == 0) == isgcmd && This depends on the value of isgcmd being 0 or 1. It obviously is, but we have to examine all the callers (just 1) to see this. The bool declaration lets the compiler ensure this. You can ensure it directly replacing isgcmd by (isgcmd != 0). Maybe I wouldn't object to using bool if you rewrote ed to use it in all places that use booleans. ed doesn't seem to have many such places. It uses the variable "int gflag" a lot. gflag contains 5 flags. These are essentially packed booleans. It would be silly to use booleans for the easy case of 1 truth value while continuing to use ints for more complicated cases, but I think that if you translate the complicated cases to use booleans then they would be even more complicated. Bruce