Date: Tue, 21 Apr 2015 05:53:09 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Eitan Adler <eadler@freebsd.org> Cc: Bruce Evans <brde@optusnet.com.au>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r281758 - head/bin/ed Message-ID: <20150421050859.H11701@besplex.bde.org> In-Reply-To: <CAF6rxgmpszvBxmWwLCUbL_s_68F5jB4ogSogOyLEZ4ZZcO-zag@mail.gmail.com> References: <201504200207.t3K27vFt078041@svn.freebsd.org> <20150420134409.I855@besplex.bde.org> <CAF6rxgmpszvBxmWwLCUbL_s_68F5jB4ogSogOyLEZ4ZZcO-zag@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 20 Apr 2015, Eitan Adler wrote: > On 19 April 2015 at 21:23, Bruce Evans <brde@optusnet.com.au> 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 <limits.h> > #include <regex.h> > #include <signal.h> > +#include <stdbool.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> 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 <stdbool.h> 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 <stdio.h> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150421050859.H11701>