Re: [patch] security(8) and spamd blacklist
On Thu, Jun 29, 2017 at 10:06:56PM +0100, Stuart Henderson wrote: > On 2017/06/29 21:37, Fritjof Bornebusch wrote: > > Hi, > > > > security(8) iterates over /var/mail and check is the files belong to the > > owner of the same name. So far so good, but spamd.conf.5 says: > > > > override:\ > > :white:\ > > :method=file:\ > > :file=/var/mail/override.txt: > > > > myblack:\ > > :black:\ > > :msg=/var/mail/myblackmsg.txt:\ > > :method=file:\ > > :file=/var/mail/myblack.txt: > > > > so the user *black.txt* and/or *override.txt* are assumed to exist > > by security(8). As it says: > > > > Checking mailbox ownership. > > user myblack.txt mailbox is owned by _spamd > > > > The following diff documents this in the manpage of spamd.conf(5) by > > changing the path to /var/mail/_spamd/. > > > > I thought about changing security(8) to fix this, but _spamd is the name > > of the user, so it does whats it's supposed to do. > > Wouldn't something like /etc/mail be better for these examples? > > It seems contradictory to hier(7) to have anything other than user mailboxes > in /var/mail (even if it's just an example in the manual). > Good point. Index: spamd.conf.5 === RCS file: /cvs/src/share/man/man5/spamd.conf.5,v retrieving revision 1.19 diff -u -p -r1.19 spamd.conf.5 --- spamd.conf.516 Mar 2017 15:09:32 - 1.19 +++ spamd.conf.529 Jun 2017 21:24:02 - @@ -65,13 +65,13 @@ nixspam:\e override:\e :white:\e :method=file:\e - :file=/var/mail/override.txt: + :file=/etc/mail/override.txt: myblack:\e :black:\e - :msg=/var/mail/myblackmsg.txt:\e + :msg=/etc/mail/myblackmsg.txt:\e :method=file:\e - :file=/var/mail/myblack.txt: + :file=/etc/mail/myblack.txt: .Ed .Pp The default configuration file must include the entry
[patch] security(8) and spamd blacklist
Hi, security(8) iterates over /var/mail and check is the files belong to the owner of the same name. So far so good, but spamd.conf.5 says: override:\ :white:\ :method=file:\ :file=/var/mail/override.txt: myblack:\ :black:\ :msg=/var/mail/myblackmsg.txt:\ :method=file:\ :file=/var/mail/myblack.txt: so the user *black.txt* and/or *override.txt* are assumed to exist by security(8). As it says: Checking mailbox ownership. user myblack.txt mailbox is owned by _spamd The following diff documents this in the manpage of spamd.conf(5) by changing the path to /var/mail/_spamd/. I thought about changing security(8) to fix this, but _spamd is the name of the user, so it does whats it's supposed to do. Comments? Because the notification above is very annoying. --f. Index: spamd.conf.5 === RCS file: /cvs/src/share/man/man5/spamd.conf.5,v retrieving revision 1.19 diff -u -p -r1.19 spamd.conf.5 --- spamd.conf.516 Mar 2017 15:09:32 - 1.19 +++ spamd.conf.529 Jun 2017 19:30:27 - @@ -65,13 +65,13 @@ nixspam:\e override:\e :white:\e :method=file:\e - :file=/var/mail/override.txt: + :file=/var/mail/_spamd/override.txt: myblack:\e :black:\e - :msg=/var/mail/myblackmsg.txt:\e + :msg=/var/mail/_spamd/myblackmsg.txt:\e :method=file:\e - :file=/var/mail/myblack.txt: + :file=/var/mail/_spamd/myblack.txt: .Ed .Pp The default configuration file must include the entry
Re: [patch] return instead of exit(3) in src/bin/
On Mon, Aug 31, 2015 at 10:59:36PM +0200, Fritjof Bornebusch wrote: Ping > On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote: > > As suggested by deraadt@ and tobias@ it might be better to use the *return* > > statement instead of exit(3) > > inside the *main* function, to let the stack protector do its work. > > > > This diff removes such calls in all *src/bin/* tools, except those who > > already use it. > > I think I didn't miss a call and didn't introduce any bugs. > > > > New diff with help from tobias@, as theo@ pointed me to a downside. > > > --F. > > > > > > Index: cat/cat.c > === > RCS file: /cvs/src/bin/cat/cat.c,v > retrieving revision 1.21 > diff -u -p -r1.21 cat.c > --- cat/cat.c 16 Jan 2015 06:39:28 - 1.21 > +++ cat/cat.c 31 Aug 2015 20:44:20 - > @@ -103,8 +103,7 @@ main(int argc, char *argv[]) > raw_args(argv); > if (fclose(stdout)) > err(1, "stdout"); > - exit(rval); > - /* NOTREACHED */ > + return (rval); > } > > void > Index: chio/chio.c > === > RCS file: /cvs/src/bin/chio/chio.c,v > retrieving revision 1.25 > diff -u -p -r1.25 chio.c > --- chio/chio.c 16 Mar 2014 18:38:30 - 1.25 > +++ chio/chio.c 31 Aug 2015 20:44:21 - > @@ -148,7 +148,7 @@ main(int argc, char *argv[]) > if (commands[i].cc_name == NULL) > errx(1, "unknown command: %s", *argv); > > - exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); > + return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); > } > > static int > Index: chmod/chmod.c > === > RCS file: /cvs/src/bin/chmod/chmod.c,v > retrieving revision 1.34 > diff -u -p -r1.34 chmod.c > --- chmod/chmod.c 25 Jun 2015 02:04:08 - 1.34 > +++ chmod/chmod.c 31 Aug 2015 20:44:22 - > @@ -279,7 +279,7 @@ done: > if (errno) > err(1, "fts_read"); > fts_close(ftsp); > - exit(rval); > + return (rval); > } > > /* > Index: cp/cp.c > === > RCS file: /cvs/src/bin/cp/cp.c,v > retrieving revision 1.38 > diff -u -p -r1.38 cp.c > --- cp/cp.c 7 May 2015 17:32:20 - 1.38 > +++ cp/cp.c 31 Aug 2015 20:44:22 - > @@ -224,7 +224,7 @@ main(int argc, char *argv[]) > type = FILE_TO_DIR; > } > > - exit(copy(argv, type, fts_options)); > + return (copy(argv, type, fts_options)); > } > > char * > Index: date/date.c > === > RCS file: /cvs/src/bin/date/date.c,v > retrieving revision 1.47 > diff -u -p -r1.47 date.c > --- date/date.c 17 Apr 2015 16:47:47 - 1.47 > +++ date/date.c 31 Aug 2015 20:44:22 - > @@ -143,7 +143,7 @@ main(int argc, char *argv[]) > errx(1, "conversion error"); > (void)strftime(buf, sizeof(buf), format, tp); > (void)printf("%s\n", buf); > - exit(0); > + return (0); > } > > #define ATOI2(ar) ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - > '0')) > Index: dd/dd.c > === > RCS file: /cvs/src/bin/dd/dd.c,v > retrieving revision 1.21 > diff -u -p -r1.21 dd.c > --- dd/dd.c 16 Jan 2015 06:39:31 - 1.21 > +++ dd/dd.c 31 Aug 2015 20:44:22 - > @@ -85,7 +85,7 @@ main(int argc, char *argv[]) > } > > dd_close(); > - exit(0); > + return (0); > } > > static void > Index: df/df.c > === > RCS file: /cvs/src/bin/df/df.c,v > retrieving revision 1.52 > diff -u -p -r1.52 df.c > --- df/df.c 16 Jan 2015 06:39:31 - 1.52 > +++ df/df.c 31 Aug 2015 20:44:23 - > @@ -175,7 +175,7 @@ main(int argc, char *argv[]) > bsdprint(mntbuf, mntsize, maxwidth); > } > > - exit(mntsize ? 0 : 1); > + return (mntsize ? 0 : 1); > } > > char * > Index: domainname/domainname.c > === > RCS file: /cvs/src/bin/domainname/domainname.c,v > retrieving revision 1.9 > diff -u -p -r1.9 domainname.c > --- domainname/domainname.c 16 Jan 2015 06:39:31 - 1.9 > +++ domainname/domainname.c
Re: [patch]rcs: usage functions above the main ones
On Mon, Jun 15, 2015 at 11:42:10AM +0100, Nicholas Marriott wrote: Ping ... > > this seems fine to me > > > On Sun, Jun 14, 2015 at 10:38:40PM +0200, Fritjof Bornebusch wrote: > > Hi tech@, > > > > most of the tools implements the *usage* function above the *main* function. > > This patch makes it more consistent to these tools and where the different > > *usage* > > functions are implemented in rcs in general. > > > > Any comments? > > > > Regards, > > --F. > > > > > > Index: co.c > > === > > RCS file: /cvs/src/usr.bin/rcs/co.c,v > > retrieving revision 1.121 > > diff -u -p -r1.121 co.c > > --- co.c13 Jun 2015 20:15:21 - 1.121 > > +++ co.c14 Jun 2015 20:21:41 - > > @@ -43,6 +43,17 @@ static void checkout_err_nobranch(RCSFIL > > const char *, int); > > static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *); > > > > +__dead void > > +checkout_usage(void) > > +{ > > + fprintf(stderr, > > + "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n" > > + " [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n" > > + " [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n"); > > + > > + exit(1); > > +} > > + > > int > > checkout_main(int argc, char **argv) > > { > > @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv) > > } > > > > return (ret); > > -} > > - > > -__dead void > > -checkout_usage(void) > > -{ > > - fprintf(stderr, > > - "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n" > > - " [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n" > > - " [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n"); > > - > > - exit(1); > > } > > > > /* > > Index: ident.c > > === > > RCS file: /cvs/src/usr.bin/rcs/ident.c,v > > retrieving revision 1.30 > > diff -u -p -r1.30 ident.c > > --- ident.c 2 Oct 2014 06:23:15 - 1.30 > > +++ ident.c 14 Jun 2015 20:21:41 - > > @@ -41,6 +41,14 @@ static int flags = 0; > > static voidident_file(const char *, FILE *); > > static voidident_line(FILE *); > > > > +__dead void > > +ident_usage(void) > > +{ > > + fprintf(stderr, "usage: ident [-qV] [file ...]\n"); > > + > > + exit(1); > > +} > > + > > int > > ident_main(int argc, char **argv) > > { > > @@ -158,12 +166,4 @@ ident_line(FILE *fp) > > out: > > if (bp != NULL) > > buf_free(bp); > > -} > > - > > -__dead void > > -ident_usage(void) > > -{ > > - fprintf(stderr, "usage: ident [-qV] [file ...]\n"); > > - > > - exit(1); > > } > > Index: merge.c > > === > > RCS file: /cvs/src/usr.bin/rcs/merge.c,v > > retrieving revision 1.9 > > diff -u -p -r1.9 merge.c > > --- merge.c 10 Oct 2014 08:15:25 - 1.9 > > +++ merge.c 14 Jun 2015 20:21:41 - > > @@ -32,6 +32,15 @@ > > #include "rcsprog.h" > > #include "diff.h" > > > > +__dead void > > +merge_usage(void) > > +{ > > + fprintf(stderr, > > + "usage: merge [-EepqV] [-L label] file1 file2 file3\n"); > > + > > + exit(D_ERROR); > > +} > > + > > int > > merge_main(int argc, char **argv) > > { > > @@ -108,13 +117,4 @@ merge_main(int argc, char **argv) > > buf_free(bp); > > > > return (status); > > -} > > - > > -__dead void > > -merge_usage(void) > > -{ > > - (void)fprintf(stderr, > > - "usage: merge [-EepqV] [-L label] file1 file2 file3\n"); > > - > > - exit(D_ERROR); > > } > > Index: rcsclean.c > > === > > RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v > > retrieving revision 1.54 > > diff -u -p -r1.54 rcsclean.c > > --- rcsclean.c 16 Jan 2015 06:40:11 - 1.54 > > +++ rcsclean.c 14 Jun 2015 20:21:41 - > > @@ -43,6 +43,16 @@ static int uflag = 0; > > static int flags = 0; > &g
[frit...@alokat.org: Re: [patch] lpr atoi -> strtonum]
- Forwarded message from Fritjof Bornebusch <frit...@alokat.org> - Date: Sat, 26 Sep 2015 22:00:58 +0200 From: Fritjof Bornebusch <frit...@alokat.org> To: Michael Reed <m.r...@mykolab.com> Cc: tech@openbsd.org Subject: Re: [patch] lpr atoi -> strtonum On Fri, Sep 25, 2015 at 02:23:21PM -0400, Michael Reed wrote: Ping > Hi Fritjof, > Hi Michael, > I left one comment inline. > thanks. > On 09/25/15 08:18, Fritjof Bornebusch wrote: > > Hi, > > > > change atoi(3) -> strtonum(3) in lpr(1) and lprm(1). > > lprm(1) avoids negative numbers to be the first argument by using getopt(3), > > but supported values like 2.2. > > > > --F. > > > > > > Index: lpr/lpr.c > > === > > RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v > > retrieving revision 1.48 > > diff -u -p -r1.48 lpr.c > > --- lpr/lpr.c 9 Feb 2015 23:00:14 - 1.48 > > +++ lpr/lpr.c 25 Sep 2015 12:08:57 - > > @@ -112,6 +112,7 @@ main(int argc, char **argv) > > char buf[PATH_MAX]; > > int i, f, ch; > > struct stat stb; > > + const char *errstr; > > > > /* > > * Simulate setuid daemon w/ PRIV_END called. > > @@ -145,11 +146,11 @@ main(int argc, char **argv) > > switch (ch) { > > > > case '#': /* n copies */ > > - if (isdigit((unsigned char)*optarg)) { > > - i = atoi(optarg); > > - if (i > 0) > > - ncopies = i; > > - } > > + i = strtonum(optarg, 0, INT_MAX, ); > > + if (errstr) > > + errx(1, "invalid quantity number"); > > + if (i > 0) > > + ncopies = i; > > I might be missing something, but why silently allow -#0 ? The default value is 1 and if -#0 is called, this default value is used. Disallow -#0 silently makes this default value useless. And other BSD versions of lpr(1) uses this default value: https://svnweb.freebsd.org/base/head/usr.sbin/lpr/lpr/lpr.c?revision=275855=co http://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/usr.sbin/lpr/lpr/lpr.c http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/usr.sbin/lpr/lpr/lpr.c?rev=1.46=text/plain_with_tag=MAIN So I think this behavior should not be changed. The above versions redirect negative values to 1 as well, but calling -#-1 looks wrong (what should lpr(1) print, if -1 copies are requested), so I think starting from 0 is okay. > Besides that, this isn't as informative as it could be IMO; perhaps > this is better: > > case '#': /* n copies */ > ncopies = strtonum(optarg, 1, INT_MAX, ); > if (errstr) > errx(1, "number of copies %s: %s", errstr, optarg); > break; > > > break; > > > > case '4': /* troff fonts */ > > @@ -203,7 +204,9 @@ main(int argc, char **argv) > > > > case 'i': /* indent output */ > > iflag++; > > - indent = atoi(optarg); > > + indent = strtonum(optarg, 0, INT_MAX, ); > > + if (errstr) > > + errx(1, "invalid number"); > > if (indent < 0) > > indent = 8; > > break; > > Index: lprm/lprm.c > > === > > RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v > > retrieving revision 1.21 > > diff -u -p -r1.21 lprm.c > > --- lprm/lprm.c 16 Jan 2015 06:40:18 - 1.21 > > +++ lprm/lprm.c 25 Sep 2015 12:08:57 - > > @@ -77,8 +77,9 @@ main(int argc, char **argv) > > { > > struct passwd *pw; > > char *cp; > > + const char *errstr; > > long l; > > - int ch; > > + int ch, i; > > > > /* > > * Simulate setuid daemon w/ PRIV_END called. > > @@ -135,7 +136,10 @@ main(int argc, char **argv) > > if (isdigit((unsigned char)*argv[0])) { > > if (requests >= MAXREQUESTS) > > fatal("Too many requests"); > > - requ[requests++] = atoi(argv[0]); > > + i = strtonum(argv[0], 0, INT_MAX, ); > > + if (errstr) > > + fatal("invalid job number"); > > + requ[requests++] = i; > > } else { > > if (users >= MAXUSERS) > > fatal("Too many users"); > > > - End forwarded message -
Re: [patch]rcs: mark unlink as (void)
On Mon, Jun 15, 2015 at 09:56:18PM +0200, Fritjof Bornebusch wrote: Ping ... > Hi tech@, > > mark this unlink(2) call as *(void)*, as there is no need to check the return > value. > This makes it more consistent to all other unlink(2) calls, since they are > marked as *(void)* as > well. > > Regards, > --F. > > > Index: co.c > === > RCS file: /cvs/src/usr.bin/rcs/co.c,v > retrieving revision 1.121 > diff -u -p -r1.121 co.c > --- co.c 13 Jun 2015 20:15:21 - 1.121 > +++ co.c 15 Jun 2015 19:50:12 - > @@ -553,7 +553,7 @@ checkout_file_has_diffs(RCSFILE *rfp, RC > ret = diffreg(dst, tempfile, bp, D_FORCEASCII); > > buf_free(bp); > - unlink(tempfile); > + (void)unlink(tempfile); > free(tempfile); > > return (ret); >
Re: [patch]apmd ? sign
On Wed, May 20, 2015 at 05:08:21PM +0200, Fritjof Bornebusch wrote: Ping > > Index: apmd.c > === > RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v > retrieving revision 1.75 > diff -u -p -r1.75 apmd.c > --- apmd.c6 Feb 2015 08:16:50 - 1.75 > +++ apmd.c20 May 2015 15:04:38 - > @@ -403,7 +403,6 @@ main(int argc, char *argv[]) > doperf = PERF_MANUAL; > setperfpolicy("high"); > break; > - case '?': > default: > usage(); > }
Re: [patch]diff: uninitialized values
On Wed, Jun 17, 2015 at 09:19:28PM +0200, Fritjof Bornebusch wrote: > On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote: > > Hi tech@, > > > > *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called. > > > > Such initializers hiding a false positive, cause the compiler does not > understand this case can never happen. > -> warning: 'edp1' may be used uninitialized in this function > -> warning: 'edp2' may be used uninitialized in this function > > Sorry for beeing not that clear. > Ping > > Regards, > > --F. > > > > > > Index: diffdir.c > > === > > RCS file: /cvs/src/usr.bin/diff/diffdir.c,v > > retrieving revision 1.43 > > diff -u -p -r1.43 diffdir.c > > --- diffdir.c 16 Jan 2015 06:40:07 - 1.43 > > +++ diffdir.c 17 Jun 2015 18:50:57 - > > @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char > > void > > diffdir(char *p1, char *p2, int flags) > > { > > - struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL; > > - struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL; > > + struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL; > > + struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL; > > size_t dirlen1, dirlen2; > > char path1[PATH_MAX], path2[PATH_MAX]; > > int pos; > > >
Re: [patch] lpr atoi -> strtonum
On Fri, Sep 25, 2015 at 02:23:21PM -0400, Michael Reed wrote: > Hi Fritjof, > Hi Michael, > I left one comment inline. > thanks. > On 09/25/15 08:18, Fritjof Bornebusch wrote: > > Hi, > > > > change atoi(3) -> strtonum(3) in lpr(1) and lprm(1). > > lprm(1) avoids negative numbers to be the first argument by using getopt(3), > > but supported values like 2.2. > > > > --F. > > > > > > Index: lpr/lpr.c > > === > > RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v > > retrieving revision 1.48 > > diff -u -p -r1.48 lpr.c > > --- lpr/lpr.c 9 Feb 2015 23:00:14 - 1.48 > > +++ lpr/lpr.c 25 Sep 2015 12:08:57 - > > @@ -112,6 +112,7 @@ main(int argc, char **argv) > > char buf[PATH_MAX]; > > int i, f, ch; > > struct stat stb; > > + const char *errstr; > > > > /* > > * Simulate setuid daemon w/ PRIV_END called. > > @@ -145,11 +146,11 @@ main(int argc, char **argv) > > switch (ch) { > > > > case '#': /* n copies */ > > - if (isdigit((unsigned char)*optarg)) { > > - i = atoi(optarg); > > - if (i > 0) > > - ncopies = i; > > - } > > + i = strtonum(optarg, 0, INT_MAX, ); > > + if (errstr) > > + errx(1, "invalid quantity number"); > > + if (i > 0) > > + ncopies = i; > > I might be missing something, but why silently allow -#0 ? The default value is 1 and if -#0 is called, this default value is used. Disallow -#0 silently makes this default value useless. And other BSD versions of lpr(1) uses this default value: https://svnweb.freebsd.org/base/head/usr.sbin/lpr/lpr/lpr.c?revision=275855=co http://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/usr.sbin/lpr/lpr/lpr.c http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/usr.sbin/lpr/lpr/lpr.c?rev=1.46=text/plain_with_tag=MAIN So I think this behavior should not be changed. The above versions redirect negative values to 1 as well, but calling -#-1 looks wrong (what should lpr(1) print, if -1 copies are requested), so I think starting from 0 is okay. > Besides that, this isn't as informative as it could be IMO; perhaps > this is better: > > case '#': /* n copies */ > ncopies = strtonum(optarg, 1, INT_MAX, ); > if (errstr) > errx(1, "number of copies %s: %s", errstr, optarg); > break; > > > break; > > > > case '4': /* troff fonts */ > > @@ -203,7 +204,9 @@ main(int argc, char **argv) > > > > case 'i': /* indent output */ > > iflag++; > > - indent = atoi(optarg); > > + indent = strtonum(optarg, 0, INT_MAX, ); > > + if (errstr) > > + errx(1, "invalid number"); > > if (indent < 0) > > indent = 8; > > break; > > Index: lprm/lprm.c > > === > > RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v > > retrieving revision 1.21 > > diff -u -p -r1.21 lprm.c > > --- lprm/lprm.c 16 Jan 2015 06:40:18 - 1.21 > > +++ lprm/lprm.c 25 Sep 2015 12:08:57 - > > @@ -77,8 +77,9 @@ main(int argc, char **argv) > > { > > struct passwd *pw; > > char *cp; > > + const char *errstr; > > long l; > > - int ch; > > + int ch, i; > > > > /* > > * Simulate setuid daemon w/ PRIV_END called. > > @@ -135,7 +136,10 @@ main(int argc, char **argv) > > if (isdigit((unsigned char)*argv[0])) { > > if (requests >= MAXREQUESTS) > > fatal("Too many requests"); > > - requ[requests++] = atoi(argv[0]); > > + i = strtonum(argv[0], 0, INT_MAX, ); > > + if (errstr) > > + fatal("invalid job number"); > > + requ[requests++] = i; > > } else { > > if (users >= MAXUSERS) > > fatal("Too many users"); > > >
[patch] lpr style
Hi, this diff changes the following: - exit(3) to return at the end of main functions - use /* NOTREACHED */ were it belongs according to style(9) - lpc.c and lpd.c lack a return at the end of the main functions, as the main loops exists the program. I'm not sure if this is a "coders choise" argument, so correct me if I'm wrong. --F. Index: filters/lpf.c === RCS file: /cvs/src/usr.sbin/lpr/filters/lpf.c,v retrieving revision 1.13 diff -u -p -r1.13 lpf.c --- filters/lpf.c 9 Feb 2015 23:00:14 - 1.13 +++ filters/lpf.c 25 Sep 2015 11:14:02 - @@ -98,6 +98,7 @@ main(int argc, char **argv) break; default: usage(); + /* NOTREACHED */ } } argc -= optind; @@ -206,7 +207,8 @@ main(int argc, char **argv) freopen(acctfile, "a", stdout) != NULL) { printf("%7.2f\t%s:%s\n", (float)npages, host, name); } - exit(0); + + return (0); } __dead void Index: lpc/lpc.c === RCS file: /cvs/src/usr.sbin/lpr/lpc/lpc.c,v retrieving revision 1.19 diff -u -p -r1.19 lpc.c --- lpc/lpc.c 16 Jan 2015 06:40:18 - 1.19 +++ lpc/lpc.c 25 Sep 2015 11:14:02 - @@ -103,6 +103,8 @@ main(int argc, char **argv) signal(SIGINT, intr); for (;;) cmdscanner(); + + return 0; } volatile sig_atomic_t gotintr; Index: lpd/lpd.c === RCS file: /cvs/src/usr.sbin/lpr/lpd/lpd.c,v retrieving revision 1.58 diff -u -p -r1.58 lpd.c --- lpd/lpd.c 9 Feb 2015 23:00:14 - 1.58 +++ lpd/lpd.c 25 Sep 2015 11:14:02 - @@ -203,7 +203,7 @@ main(int argc, char **argv) break; default: usage(); - break; + /* NOTREACHED */ } } argc -= optind; @@ -223,6 +223,7 @@ main(int argc, char **argv) break; default: usage(); + /* NOTREACHED */ } #ifndef DEBUG @@ -419,6 +420,8 @@ main(int argc, char **argv) } (void)close(s); } + + return 0; } static void Index: lpq/lpq.c === RCS file: /cvs/src/usr.sbin/lpr/lpq/lpq.c,v retrieving revision 1.22 diff -u -p -r1.22 lpq.c --- lpq/lpq.c 9 Feb 2015 23:00:14 - 1.22 +++ lpq/lpq.c 25 Sep 2015 11:14:02 - @@ -108,6 +108,7 @@ main(int argc, char **argv) case '?': default: usage(); + /* NOTREACHED */ } } @@ -145,7 +146,8 @@ main(int argc, char **argv) } } else displayq(lflag); - exit(0); + + return (0); } /* XXX - could be common w/ lpd */ Index: lpr/lpr.c === RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v retrieving revision 1.48 diff -u -p -r1.48 lpr.c --- lpr/lpr.c 9 Feb 2015 23:00:14 - 1.48 +++ lpr/lpr.c 25 Sep 2015 11:14:02 - @@ -238,6 +238,7 @@ main(int argc, char **argv) default: usage(); + /* NOTREACHED */ } } argc -= optind; @@ -389,7 +390,6 @@ main(int argc, char **argv) } cleanup(0); return (1); - /* NOTREACHED */ } /* Index: lprm/lprm.c === RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v retrieving revision 1.21 diff -u -p -r1.21 lprm.c --- lprm/lprm.c 16 Jan 2015 06:40:18 - 1.21 +++ lprm/lprm.c 25 Sep 2015 11:14:02 - @@ -122,6 +122,7 @@ main(int argc, char **argv) break; default: usage(); + /* NOTREACHED */ } } argc -= optind; @@ -146,7 +147,7 @@ main(int argc, char **argv) } rmjob(); - exit(0); + return (0); } static __dead void Index: lptest/lptest.c === RCS file: /cvs/src/usr.sbin/lpr/lptest/lptest.c,v retrieving revision 1.8 diff -u -p -r1.8 lptest.c --- lptest/lptest.c 27 Oct 2009 23:59:52 - 1.8 +++ lptest/lptest.c 25 Sep 2015 11:14:02 - @@ -66,5 +66,5 @@ main(int argc, char **argv) putchar('\n'); } (void)fflush(stdout); - exit(0); + return (0); } Index: pac/pac.c === RCS file: /cvs/src/usr.sbin/lpr/pac/pac.c,v retrieving revision 1.25 diff -u -p -r1.25 pac.c --- pac/pac.c
[patch] lpr atoi -> strtonum
Hi, change atoi(3) -> strtonum(3) in lpr(1) and lprm(1). lprm(1) avoids negative numbers to be the first argument by using getopt(3), but supported values like 2.2. --F. Index: lpr/lpr.c === RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v retrieving revision 1.48 diff -u -p -r1.48 lpr.c --- lpr/lpr.c 9 Feb 2015 23:00:14 - 1.48 +++ lpr/lpr.c 25 Sep 2015 12:08:57 - @@ -112,6 +112,7 @@ main(int argc, char **argv) char buf[PATH_MAX]; int i, f, ch; struct stat stb; + const char *errstr; /* * Simulate setuid daemon w/ PRIV_END called. @@ -145,11 +146,11 @@ main(int argc, char **argv) switch (ch) { case '#': /* n copies */ - if (isdigit((unsigned char)*optarg)) { - i = atoi(optarg); - if (i > 0) - ncopies = i; - } + i = strtonum(optarg, 0, INT_MAX, ); + if (errstr) + errx(1, "invalid quantity number"); + if (i > 0) + ncopies = i; break; case '4': /* troff fonts */ @@ -203,7 +204,9 @@ main(int argc, char **argv) case 'i': /* indent output */ iflag++; - indent = atoi(optarg); + indent = strtonum(optarg, 0, INT_MAX, ); + if (errstr) + errx(1, "invalid number"); if (indent < 0) indent = 8; break; Index: lprm/lprm.c === RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v retrieving revision 1.21 diff -u -p -r1.21 lprm.c --- lprm/lprm.c 16 Jan 2015 06:40:18 - 1.21 +++ lprm/lprm.c 25 Sep 2015 12:08:57 - @@ -77,8 +77,9 @@ main(int argc, char **argv) { struct passwd *pw; char *cp; + const char *errstr; long l; - int ch; + int ch, i; /* * Simulate setuid daemon w/ PRIV_END called. @@ -135,7 +136,10 @@ main(int argc, char **argv) if (isdigit((unsigned char)*argv[0])) { if (requests >= MAXREQUESTS) fatal("Too many requests"); - requ[requests++] = atoi(argv[0]); + i = strtonum(argv[0], 0, INT_MAX, ); + if (errstr) + fatal("invalid job number"); + requ[requests++] = i; } else { if (users >= MAXUSERS) fatal("Too many users");
Re: [patch]diff: uninitialized values
On Wed, Jun 17, 2015 at 09:19:28PM +0200, Fritjof Bornebusch wrote: > On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote: > > Hi tech@, > > > > *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called. > > > > Such initializers hiding a false positive, cause the compiler does not > understand this case can never happen. > -> warning: 'edp1' may be used uninitialized in this function > -> warning: 'edp2' may be used uninitialized in this function > > Sorry for beeing not that clear. > Ping > > Regards, > > --F. > > > > > > Index: diffdir.c > > === > > RCS file: /cvs/src/usr.bin/diff/diffdir.c,v > > retrieving revision 1.43 > > diff -u -p -r1.43 diffdir.c > > --- diffdir.c 16 Jan 2015 06:40:07 - 1.43 > > +++ diffdir.c 17 Jun 2015 18:50:57 - > > @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char > > void > > diffdir(char *p1, char *p2, int flags) > > { > > - struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL; > > - struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL; > > + struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL; > > + struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL; > > size_t dirlen1, dirlen2; > > char path1[PATH_MAX], path2[PATH_MAX]; > > int pos; > > >
Re: [patch]rcs: mark unlink as (void)
On Mon, Jun 15, 2015 at 09:56:18PM +0200, Fritjof Bornebusch wrote: > Hi tech@, > > mark this unlink(2) call as *(void)*, as there is no need to check the return > value. > This makes it more consistent to all other unlink(2) calls, since they are > marked as *(void)* as > well. > > Regards, > --F. > Ping > > Index: co.c > === > RCS file: /cvs/src/usr.bin/rcs/co.c,v > retrieving revision 1.121 > diff -u -p -r1.121 co.c > --- co.c 13 Jun 2015 20:15:21 - 1.121 > +++ co.c 15 Jun 2015 19:50:12 - > @@ -553,7 +553,7 @@ checkout_file_has_diffs(RCSFILE *rfp, RC > ret = diffreg(dst, tempfile, bp, D_FORCEASCII); > > buf_free(bp); > - unlink(tempfile); > + (void)unlink(tempfile); > free(tempfile); > > return (ret); >
Re: [patch]rcs: usage functions above the main ones
On Mon, Jun 15, 2015 at 11:42:10AM +0100, Nicholas Marriott wrote: > > this seems fine to me > Ping ... > > On Sun, Jun 14, 2015 at 10:38:40PM +0200, Fritjof Bornebusch wrote: > > Hi tech@, > > > > most of the tools implements the *usage* function above the *main* function. > > This patch makes it more consistent to these tools and where the different > > *usage* > > functions are implemented in rcs in general. > > > > Any comments? > > > > Regards, > > --F. > > > > > > Index: co.c > > === > > RCS file: /cvs/src/usr.bin/rcs/co.c,v > > retrieving revision 1.121 > > diff -u -p -r1.121 co.c > > --- co.c13 Jun 2015 20:15:21 - 1.121 > > +++ co.c14 Jun 2015 20:21:41 - > > @@ -43,6 +43,17 @@ static void checkout_err_nobranch(RCSFIL > > const char *, int); > > static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *); > > > > +__dead void > > +checkout_usage(void) > > +{ > > + fprintf(stderr, > > + "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n" > > + " [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n" > > + " [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n"); > > + > > + exit(1); > > +} > > + > > int > > checkout_main(int argc, char **argv) > > { > > @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv) > > } > > > > return (ret); > > -} > > - > > -__dead void > > -checkout_usage(void) > > -{ > > - fprintf(stderr, > > - "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n" > > - " [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n" > > - " [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n"); > > - > > - exit(1); > > } > > > > /* > > Index: ident.c > > === > > RCS file: /cvs/src/usr.bin/rcs/ident.c,v > > retrieving revision 1.30 > > diff -u -p -r1.30 ident.c > > --- ident.c 2 Oct 2014 06:23:15 - 1.30 > > +++ ident.c 14 Jun 2015 20:21:41 - > > @@ -41,6 +41,14 @@ static int flags = 0; > > static voidident_file(const char *, FILE *); > > static voidident_line(FILE *); > > > > +__dead void > > +ident_usage(void) > > +{ > > + fprintf(stderr, "usage: ident [-qV] [file ...]\n"); > > + > > + exit(1); > > +} > > + > > int > > ident_main(int argc, char **argv) > > { > > @@ -158,12 +166,4 @@ ident_line(FILE *fp) > > out: > > if (bp != NULL) > > buf_free(bp); > > -} > > - > > -__dead void > > -ident_usage(void) > > -{ > > - fprintf(stderr, "usage: ident [-qV] [file ...]\n"); > > - > > - exit(1); > > } > > Index: merge.c > > === > > RCS file: /cvs/src/usr.bin/rcs/merge.c,v > > retrieving revision 1.9 > > diff -u -p -r1.9 merge.c > > --- merge.c 10 Oct 2014 08:15:25 - 1.9 > > +++ merge.c 14 Jun 2015 20:21:41 - > > @@ -32,6 +32,15 @@ > > #include "rcsprog.h" > > #include "diff.h" > > > > +__dead void > > +merge_usage(void) > > +{ > > + fprintf(stderr, > > + "usage: merge [-EepqV] [-L label] file1 file2 file3\n"); > > + > > + exit(D_ERROR); > > +} > > + > > int > > merge_main(int argc, char **argv) > > { > > @@ -108,13 +117,4 @@ merge_main(int argc, char **argv) > > buf_free(bp); > > > > return (status); > > -} > > - > > -__dead void > > -merge_usage(void) > > -{ > > - (void)fprintf(stderr, > > - "usage: merge [-EepqV] [-L label] file1 file2 file3\n"); > > - > > - exit(D_ERROR); > > } > > Index: rcsclean.c > > === > > RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v > > retrieving revision 1.54 > > diff -u -p -r1.54 rcsclean.c > > --- rcsclean.c 16 Jan 2015 06:40:11 - 1.54 > > +++ rcsclean.c 14 Jun 2015 20:21:41 - > > @@ -43,6 +43,16 @@ static int uflag = 0; > > static int flags = 0; > &g
Re: [patch] return instead of exit(3) in src/bin/
On Mon, Aug 31, 2015 at 10:59:36PM +0200, Fritjof Bornebusch wrote: > On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote: > > As suggested by deraadt@ and tobias@ it might be better to use the *return* > > statement instead of exit(3) > > inside the *main* function, to let the stack protector do its work. > > > > This diff removes such calls in all *src/bin/* tools, except those who > > already use it. > > I think I didn't miss a call and didn't introduce any bugs. > > > > New diff with help from tobias@, as theo@ pointed me to a downside. > > > --F. > > > Any comments or feedback? > > > Index: cat/cat.c > === > RCS file: /cvs/src/bin/cat/cat.c,v > retrieving revision 1.21 > diff -u -p -r1.21 cat.c > --- cat/cat.c 16 Jan 2015 06:39:28 - 1.21 > +++ cat/cat.c 31 Aug 2015 20:44:20 - > @@ -103,8 +103,7 @@ main(int argc, char *argv[]) > raw_args(argv); > if (fclose(stdout)) > err(1, "stdout"); > - exit(rval); > - /* NOTREACHED */ > + return (rval); > } > > void > Index: chio/chio.c > === > RCS file: /cvs/src/bin/chio/chio.c,v > retrieving revision 1.25 > diff -u -p -r1.25 chio.c > --- chio/chio.c 16 Mar 2014 18:38:30 - 1.25 > +++ chio/chio.c 31 Aug 2015 20:44:21 - > @@ -148,7 +148,7 @@ main(int argc, char *argv[]) > if (commands[i].cc_name == NULL) > errx(1, "unknown command: %s", *argv); > > - exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); > + return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); > } > > static int > Index: chmod/chmod.c > === > RCS file: /cvs/src/bin/chmod/chmod.c,v > retrieving revision 1.34 > diff -u -p -r1.34 chmod.c > --- chmod/chmod.c 25 Jun 2015 02:04:08 - 1.34 > +++ chmod/chmod.c 31 Aug 2015 20:44:22 - > @@ -279,7 +279,7 @@ done: > if (errno) > err(1, "fts_read"); > fts_close(ftsp); > - exit(rval); > + return (rval); > } > > /* > Index: cp/cp.c > === > RCS file: /cvs/src/bin/cp/cp.c,v > retrieving revision 1.38 > diff -u -p -r1.38 cp.c > --- cp/cp.c 7 May 2015 17:32:20 - 1.38 > +++ cp/cp.c 31 Aug 2015 20:44:22 - > @@ -224,7 +224,7 @@ main(int argc, char *argv[]) > type = FILE_TO_DIR; > } > > - exit(copy(argv, type, fts_options)); > + return (copy(argv, type, fts_options)); > } > > char * > Index: date/date.c > === > RCS file: /cvs/src/bin/date/date.c,v > retrieving revision 1.47 > diff -u -p -r1.47 date.c > --- date/date.c 17 Apr 2015 16:47:47 - 1.47 > +++ date/date.c 31 Aug 2015 20:44:22 - > @@ -143,7 +143,7 @@ main(int argc, char *argv[]) > errx(1, "conversion error"); > (void)strftime(buf, sizeof(buf), format, tp); > (void)printf("%s\n", buf); > - exit(0); > + return (0); > } > > #define ATOI2(ar) ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - > '0')) > Index: dd/dd.c > === > RCS file: /cvs/src/bin/dd/dd.c,v > retrieving revision 1.21 > diff -u -p -r1.21 dd.c > --- dd/dd.c 16 Jan 2015 06:39:31 - 1.21 > +++ dd/dd.c 31 Aug 2015 20:44:22 - > @@ -85,7 +85,7 @@ main(int argc, char *argv[]) > } > > dd_close(); > - exit(0); > + return (0); > } > > static void > Index: df/df.c > === > RCS file: /cvs/src/bin/df/df.c,v > retrieving revision 1.52 > diff -u -p -r1.52 df.c > --- df/df.c 16 Jan 2015 06:39:31 - 1.52 > +++ df/df.c 31 Aug 2015 20:44:23 - > @@ -175,7 +175,7 @@ main(int argc, char *argv[]) > bsdprint(mntbuf, mntsize, maxwidth); > } > > - exit(mntsize ? 0 : 1); > + return (mntsize ? 0 : 1); > } > > char * > Index: domainname/domainname.c > === > RCS file: /cvs/src/bin/domainname/domainname.c,v > retrieving revision 1.9 > diff -u -p -r1.9 domainname.c > --- domainname/domainname.c 16 Jan 2015 06:39:31 - 1.9 > +++ domai
Re: [patch] return instead of exit(3) in src/bin/
On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote: > As suggested by deraadt@ and tobias@ it might be better to use the *return* > statement instead of exit(3) > inside the *main* function, to let the stack protector do its work. > > This diff removes such calls in all *src/bin/* tools, except those who > already use it. > I think I didn't miss a call and didn't introduce any bugs. > New diff with help from tobias@, as theo@ pointed me to a downside. > --F. > Index: cat/cat.c === RCS file: /cvs/src/bin/cat/cat.c,v retrieving revision 1.21 diff -u -p -r1.21 cat.c --- cat/cat.c 16 Jan 2015 06:39:28 - 1.21 +++ cat/cat.c 31 Aug 2015 20:44:20 - @@ -103,8 +103,7 @@ main(int argc, char *argv[]) raw_args(argv); if (fclose(stdout)) err(1, "stdout"); - exit(rval); - /* NOTREACHED */ + return (rval); } void Index: chio/chio.c === RCS file: /cvs/src/bin/chio/chio.c,v retrieving revision 1.25 diff -u -p -r1.25 chio.c --- chio/chio.c 16 Mar 2014 18:38:30 - 1.25 +++ chio/chio.c 31 Aug 2015 20:44:21 - @@ -148,7 +148,7 @@ main(int argc, char *argv[]) if (commands[i].cc_name == NULL) errx(1, "unknown command: %s", *argv); - exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); + return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv)); } static int Index: chmod/chmod.c === RCS file: /cvs/src/bin/chmod/chmod.c,v retrieving revision 1.34 diff -u -p -r1.34 chmod.c --- chmod/chmod.c 25 Jun 2015 02:04:08 - 1.34 +++ chmod/chmod.c 31 Aug 2015 20:44:22 - @@ -279,7 +279,7 @@ done: if (errno) err(1, "fts_read"); fts_close(ftsp); - exit(rval); + return (rval); } /* Index: cp/cp.c === RCS file: /cvs/src/bin/cp/cp.c,v retrieving revision 1.38 diff -u -p -r1.38 cp.c --- cp/cp.c 7 May 2015 17:32:20 - 1.38 +++ cp/cp.c 31 Aug 2015 20:44:22 - @@ -224,7 +224,7 @@ main(int argc, char *argv[]) type = FILE_TO_DIR; } - exit(copy(argv, type, fts_options)); + return (copy(argv, type, fts_options)); } char * Index: date/date.c === RCS file: /cvs/src/bin/date/date.c,v retrieving revision 1.47 diff -u -p -r1.47 date.c --- date/date.c 17 Apr 2015 16:47:47 - 1.47 +++ date/date.c 31 Aug 2015 20:44:22 - @@ -143,7 +143,7 @@ main(int argc, char *argv[]) errx(1, "conversion error"); (void)strftime(buf, sizeof(buf), format, tp); (void)printf("%s\n", buf); - exit(0); + return (0); } #defineATOI2(ar) ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - '0')) Index: dd/dd.c === RCS file: /cvs/src/bin/dd/dd.c,v retrieving revision 1.21 diff -u -p -r1.21 dd.c --- dd/dd.c 16 Jan 2015 06:39:31 - 1.21 +++ dd/dd.c 31 Aug 2015 20:44:22 - @@ -85,7 +85,7 @@ main(int argc, char *argv[]) } dd_close(); - exit(0); + return (0); } static void Index: df/df.c === RCS file: /cvs/src/bin/df/df.c,v retrieving revision 1.52 diff -u -p -r1.52 df.c --- df/df.c 16 Jan 2015 06:39:31 - 1.52 +++ df/df.c 31 Aug 2015 20:44:23 - @@ -175,7 +175,7 @@ main(int argc, char *argv[]) bsdprint(mntbuf, mntsize, maxwidth); } - exit(mntsize ? 0 : 1); + return (mntsize ? 0 : 1); } char * Index: domainname/domainname.c === RCS file: /cvs/src/bin/domainname/domainname.c,v retrieving revision 1.9 diff -u -p -r1.9 domainname.c --- domainname/domainname.c 16 Jan 2015 06:39:31 - 1.9 +++ domainname/domainname.c 31 Aug 2015 20:44:23 - @@ -66,7 +66,7 @@ main(int argc, char *argv[]) err(1, "getdomainname"); (void)printf("%s\n", domainname); } - exit(0); + return (0); } void Index: expr/expr.c === RCS file: /cvs/src/bin/expr/expr.c,v retrieving revision 1.20 diff -u -p -r1.20 expr.c --- expr/expr.c 11 Aug 2015 17:15:46 - 1.20 +++ expr/expr.c 31 Aug 2015 20:44:23 - @@ -518,5 +518,5 @@ main(int argc, char *argv[]) else printf("%s\n", vp->u.s); - exit(is_zero_or_null(vp)); + return (is_zero_or_null
Re: [patch]rcs: mark unlink as (void)
On Mon, Jun 15, 2015 at 09:56:18PM +0200, Fritjof Bornebusch wrote: Ping Hi tech@, mark this unlink(2) call as *(void)*, as there is no need to check the return value. This makes it more consistent to all other unlink(2) calls, since they are marked as *(void)* as well. Regards, --F. Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.121 diff -u -p -r1.121 co.c --- co.c 13 Jun 2015 20:15:21 - 1.121 +++ co.c 15 Jun 2015 19:50:12 - @@ -553,7 +553,7 @@ checkout_file_has_diffs(RCSFILE *rfp, RC ret = diffreg(dst, tempfile, bp, D_FORCEASCII); buf_free(bp); - unlink(tempfile); + (void)unlink(tempfile); free(tempfile); return (ret);
Re: [patch]rcs: usage functions above the main ones
On Mon, Jun 15, 2015 at 11:42:10AM +0100, Nicholas Marriott wrote: this seems fine to me Ping On Sun, Jun 14, 2015 at 10:38:40PM +0200, Fritjof Bornebusch wrote: Hi tech@, most of the tools implements the *usage* function above the *main* function. This patch makes it more consistent to these tools and where the different *usage* functions are implemented in rcs in general. Any comments? Regards, --F. Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.121 diff -u -p -r1.121 co.c --- co.c13 Jun 2015 20:15:21 - 1.121 +++ co.c14 Jun 2015 20:21:41 - @@ -43,6 +43,17 @@ static void checkout_err_nobranch(RCSFIL const char *, int); static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *); +__dead void +checkout_usage(void) +{ + fprintf(stderr, + usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n + [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n + [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); + + exit(1); +} + int checkout_main(int argc, char **argv) { @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv) } return (ret); -} - -__dead void -checkout_usage(void) -{ - fprintf(stderr, - usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n - [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n - [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); - - exit(1); } /* Index: ident.c === RCS file: /cvs/src/usr.bin/rcs/ident.c,v retrieving revision 1.30 diff -u -p -r1.30 ident.c --- ident.c 2 Oct 2014 06:23:15 - 1.30 +++ ident.c 14 Jun 2015 20:21:41 - @@ -41,6 +41,14 @@ static int flags = 0; static voidident_file(const char *, FILE *); static voidident_line(FILE *); +__dead void +ident_usage(void) +{ + fprintf(stderr, usage: ident [-qV] [file ...]\n); + + exit(1); +} + int ident_main(int argc, char **argv) { @@ -158,12 +166,4 @@ ident_line(FILE *fp) out: if (bp != NULL) buf_free(bp); -} - -__dead void -ident_usage(void) -{ - fprintf(stderr, usage: ident [-qV] [file ...]\n); - - exit(1); } Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.9 diff -u -p -r1.9 merge.c --- merge.c 10 Oct 2014 08:15:25 - 1.9 +++ merge.c 14 Jun 2015 20:21:41 - @@ -32,6 +32,15 @@ #include rcsprog.h #include diff.h +__dead void +merge_usage(void) +{ + fprintf(stderr, + usage: merge [-EepqV] [-L label] file1 file2 file3\n); + + exit(D_ERROR); +} + int merge_main(int argc, char **argv) { @@ -108,13 +117,4 @@ merge_main(int argc, char **argv) buf_free(bp); return (status); -} - -__dead void -merge_usage(void) -{ - (void)fprintf(stderr, - usage: merge [-EepqV] [-L label] file1 file2 file3\n); - - exit(D_ERROR); } Index: rcsclean.c === RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v retrieving revision 1.54 diff -u -p -r1.54 rcsclean.c --- rcsclean.c 16 Jan 2015 06:40:11 - 1.54 +++ rcsclean.c 14 Jun 2015 20:21:41 - @@ -43,6 +43,16 @@ static int uflag = 0; static int flags = 0; static char *locker = NULL; +__dead void +rcsclean_usage(void) +{ + fprintf(stderr, + usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n + [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n); + + exit(1); +} + int rcsclean_main(int argc, char **argv) { @@ -116,16 +126,6 @@ rcsclean_main(int argc, char **argv) rcsclean_file(argv[i], rev_str); return (0); -} - -__dead void -rcsclean_usage(void) -{ - fprintf(stderr, - usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n - [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n); - - exit(1); } static void Index: rcsdiff.c === RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v retrieving revision 1.83 diff -u -p -r1.83 rcsdiff.c --- rcsdiff.c 13 Jun 2015 20:15:21 - 1.83 +++ rcsdiff.c 14 Jun 2015 20:21:41 - @@ -45,6 +45,16 @@ static int quiet; static int kflag = RCS_KWEXP_ERR; static char *diff_ignore_pats; +__dead void +rcsdiff_usage(void) +{ + fprintf(stderr, + usage: rcsdiff [-cnquV] [-kmode] [-rrev] [-xsuffixes] [-ztz]\n
Re: [patch]rcs: rlog manpage typo
On Thu, Jun 18, 2015 at 10:49:32PM +0200, Pablo Méndez Hernández wrote: Hi, El 18/6/2015 22:46, Fritjof Bornebusch frit...@alokat.org escribiA^3: Hi tech@, *logins is omitted* sounds a little strange, doesn't it? logins as a keyword? Yes, but if you read it, it sounds wrong in some way. Regards. Pablo Regards, --F. Regards, --F. Index: rlog.1 === RCS file: /cvs/src/usr.bin/rcs/rlog.1,v retrieving revision 1.24 diff -u -p -r1.24 rlog.1 --- rlog.1A A A 3 Sep 2010 11:09:29 -A A A A 1.24 +++ rlog.1A A A 18 Jun 2015 20:41:40 - @@ -145,7 +145,7 @@ Print information about revisions checke A in a comma-separated list. A If A .Ar logins -is omitted, the user's login is assumed. +are omitted, the user's login is assumed. A .It Fl x Ns Ar suffixes A Specifies the suffixes for RCS files. A Suffixes should be separated by the
[patch]rcs: merge typo
What about this comma. I saw a few manpages, having it at this location. Regards, --F. Index: merge.1 === RCS file: /cvs/src/usr.bin/rcs/merge.1,v retrieving revision 1.3 diff -u -p -r1.3 merge.1 --- merge.1 28 Oct 2010 15:08:50 - 1.3 +++ merge.1 18 Jun 2015 20:52:30 - @@ -45,7 +45,7 @@ for details. .It Fl e Same as .Fl E -but does not warn about conflicts. +, but does not warn about conflicts. .It Fl L Ar label Specifies labels to be used in place of corresponding file names in conflict reports.
Re: [patch]rcs: ci manpage typo
On Thu, Jun 18, 2015 at 09:48:23PM +0100, Jason McIntyre wrote: On Thu, Jun 18, 2015 at 10:33:53PM +0200, Fritjof Bornebusch wrote: Hi tech@, isn't there a comma missing? depends how you like your commas. if i were writing it, i'd have the comma. but many wouldn;t, and it's certainly not wrong. the reason i like that last comma is demonstrated with this wonderful dedication: To my parents, Ayn Rand and God. love it ;) jmc ps won;t be changing the doc though. i don;t see the point of enforcing this - it;s really a matter of preference. I see. Thanks for the explanation. Regards, --F. Index: ci.1 === RCS file: /cvs/src/usr.bin/rcs/ci.1,v retrieving revision 1.38 diff -u -p -r1.38 ci.1 --- ci.112 Aug 2013 14:19:53 - 1.38 +++ ci.118 Jun 2015 20:30:22 - @@ -102,7 +102,7 @@ Only do update check-in. Print error and refuse to do check-in if the RCS file does not already exist. .It Fl k Ns Op Ar rev Search the working file for keywords and set the revision number, -creation date, state and author to the values found in these keywords +creation date, state, and author to the values found in these keywords instead of computing them. .It Fl l Ns Op Ar rev The same as
[patch]rcs: ci manpage typo
Hi tech@, isn't there a comma missing? Regards, --F. Index: ci.1 === RCS file: /cvs/src/usr.bin/rcs/ci.1,v retrieving revision 1.38 diff -u -p -r1.38 ci.1 --- ci.112 Aug 2013 14:19:53 - 1.38 +++ ci.118 Jun 2015 20:30:22 - @@ -102,7 +102,7 @@ Only do update check-in. Print error and refuse to do check-in if the RCS file does not already exist. .It Fl k Ns Op Ar rev Search the working file for keywords and set the revision number, -creation date, state and author to the values found in these keywords +creation date, state, and author to the values found in these keywords instead of computing them. .It Fl l Ns Op Ar rev The same as
[patch]rcs: rlog manpage typo
Hi tech@, *logins is omitted* sounds a little strange, doesn't it? Regards, --F. Index: rlog.1 === RCS file: /cvs/src/usr.bin/rcs/rlog.1,v retrieving revision 1.24 diff -u -p -r1.24 rlog.1 --- rlog.1 3 Sep 2010 11:09:29 - 1.24 +++ rlog.1 18 Jun 2015 20:41:40 - @@ -145,7 +145,7 @@ Print information about revisions checke in a comma-separated list. If .Ar logins -is omitted, the user's login is assumed. +are omitted, the user's login is assumed. .It Fl x Ns Ar suffixes Specifies the suffixes for RCS files. Suffixes should be separated by the
Re: [patch]rcs: rlog manpage typo
On Thu, Jun 18, 2015 at 09:57:13PM +0100, Jason McIntyre wrote: On Thu, Jun 18, 2015 at 10:44:07PM +0200, Fritjof Bornebusch wrote: Hi tech@, *logins is omitted* sounds a little strange, doesn't it? it does, because in your head you're thinking of logins as being the plural of login (i.e. more than one login). but you can also - correctly - think of logins as being the argument name. you could read the text below as: If [the argument] logins is omitted... sounds different now, eh? in the docs, there is often this overlap where we can think of the argument name in more than one way. it creates ambiguities, but there you go. it would be impractical to try and force one style, i think. still, for this reason i dislike argument names which take the plural form. Yeah, this can cause some confusion. But if it's the style, I'm okay with it. ;) jmc Regards, --F. Regards, --F. Index: rlog.1 === RCS file: /cvs/src/usr.bin/rcs/rlog.1,v retrieving revision 1.24 diff -u -p -r1.24 rlog.1 --- rlog.1 3 Sep 2010 11:09:29 - 1.24 +++ rlog.1 18 Jun 2015 20:41:40 - @@ -145,7 +145,7 @@ Print information about revisions checke in a comma-separated list. If .Ar logins -is omitted, the user's login is assumed. +are omitted, the user's login is assumed. .It Fl x Ns Ar suffixes Specifies the suffixes for RCS files. Suffixes should be separated by the
[patch]diff: xstrdup wrappes strdup(3)
Hi tech@, as requested by nicm@, xstrdup calls strdup(3) now. Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/diff/xmalloc.c,v retrieving revision 1.6 diff -u -p -r1.6 xmalloc.c --- xmalloc.c 29 Apr 2015 04:00:25 - 1.6 +++ xmalloc.c 17 Jun 2015 18:13:25 - @@ -73,12 +73,10 @@ xfree(void *ptr) char * xstrdup(const char *str) { - size_t len; char *cp; - - len = strlen(str) + 1; - cp = xmalloc(len); - strlcpy(cp, str, len); + + if ((cp = strdup(str)) == NULL) + err(1, xstrdup); return cp; }
[patch]diff: uninitialized values
Hi tech@, *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called. Regards, --F. Index: diffdir.c === RCS file: /cvs/src/usr.bin/diff/diffdir.c,v retrieving revision 1.43 diff -u -p -r1.43 diffdir.c --- diffdir.c 16 Jan 2015 06:40:07 - 1.43 +++ diffdir.c 17 Jun 2015 18:50:57 - @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char void diffdir(char *p1, char *p2, int flags) { - struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL; - struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL; + struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL; + struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL; size_t dirlen1, dirlen2; char path1[PATH_MAX], path2[PATH_MAX]; int pos;
Re: [patch]diff: uninitialized values
On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote: Hi tech@, *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called. Such initializers hiding a false positive, cause the compiler does not understand this case can never happen. - warning: 'edp1' may be used uninitialized in this function - warning: 'edp2' may be used uninitialized in this function Sorry for beeing not that clear. Regards, --F. Index: diffdir.c === RCS file: /cvs/src/usr.bin/diff/diffdir.c,v retrieving revision 1.43 diff -u -p -r1.43 diffdir.c --- diffdir.c 16 Jan 2015 06:40:07 - 1.43 +++ diffdir.c 17 Jun 2015 18:50:57 - @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char void diffdir(char *p1, char *p2, int flags) { - struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL; - struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL; + struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL; + struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL; size_t dirlen1, dirlen2; char path1[PATH_MAX], path2[PATH_MAX]; int pos;
[patch]file: xstrdup just wrappes strdup(3)
Hi tech@, as requested by nicm@, xstrdup calls strdup(3) now. Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/file/xmalloc.c,v retrieving revision 1.1 diff -u -p -r1.1 xmalloc.c --- xmalloc.c 24 Apr 2015 16:24:11 - 1.1 +++ xmalloc.c 17 Jun 2015 18:18:10 - @@ -76,13 +76,10 @@ xfree(void *ptr) char * xstrdup(const char *str) { - size_t len; char *cp; - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + if ((cp = strdup(str)) == NULL) + err(1, xstrdup); return cp; }
[patch]ssh: xstrdup wrappes strdup(3)
Hi tech@, as requested by nicm@, xstrdup just wrappes strdup(3). Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/ssh/xmalloc.c,v retrieving revision 1.32 diff -u -p -r1.32 xmalloc.c --- xmalloc.c 24 Apr 2015 01:36:01 - 1.32 +++ xmalloc.c 17 Jun 2015 18:42:43 - @@ -13,6 +13,7 @@ * called by a name other than ssh or Secure Shell. */ +#include errno.h #include stdarg.h #include stdint.h #include stdio.h @@ -66,12 +67,10 @@ xreallocarray(void *ptr, size_t nmemb, s char * xstrdup(const char *str) { - size_t len; char *cp; - len = strlen(str) + 1; - cp = xmalloc(len); - strlcpy(cp, str, len); + if ((cp = strdup(str)) == NULL) + fatal(xstrdup: %s, strerror(errno)); return cp; }
Re: [patch]rcs: xstrdup just wrappes strdup
On Mon, Jun 15, 2015 at 10:22:27AM +0100, Nicholas Marriott wrote: What about diff and ssh and file? They all use the a copy of the same xmalloc.c. Personally, I would recommend that xstrdup just calls strdup(3), as Theo said: it's 100.00% portable. But I don't think I'm the right person who should answer that question. ;) Regards, --F. On Mon, Jun 15, 2015 at 10:00:01AM +0200, Fritjof Bornebusch wrote: Hi, thanks for the hint. This one should do the trick. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.9 diff -u -p -r1.9 xmalloc.c --- xmalloc.c 13 Jun 2015 20:15:21 - 1.9 +++ xmalloc.c 15 Jun 2015 07:52:15 - @@ -68,13 +68,10 @@ xreallocarray(void *ptr, size_t nmemb, s char * xstrdup(const char *str) { - size_t len; char *cp; - - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + + if ((cp = strdup(str)) == NULL) + err(1, xstrdup); return cp; }
[patch]rcs: no null check before free(3)
Hi tech@, just saw I missed removing the null check before calling free(3), sorry. Regards, --F. Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.220 diff -u -p -r1.220 ci.c --- ci.c13 Jun 2015 20:15:21 - 1.220 +++ ci.c17 Jun 2015 07:47:09 - @@ -210,8 +210,7 @@ checkin_main(int argc, char **argv) printf(%s\n, rcs_version); exit(0); case 'w': - if (pb.author != NULL) - free(pb.author); + free(pb.author); pb.author = xstrdup(rcs_optarg); break; case 'x':
[patch]rcs: mark unlink as (void)
Hi tech@, mark this unlink(2) call as *(void)*, as there is no need to check the return value. This makes it more consistent to all other unlink(2) calls, since they are marked as *(void)* as well. Regards, --F. Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.121 diff -u -p -r1.121 co.c --- co.c13 Jun 2015 20:15:21 - 1.121 +++ co.c15 Jun 2015 19:50:12 - @@ -553,7 +553,7 @@ checkout_file_has_diffs(RCSFILE *rfp, RC ret = diffreg(dst, tempfile, bp, D_FORCEASCII); buf_free(bp); - unlink(tempfile); + (void)unlink(tempfile); free(tempfile); return (ret);
Re: [patch]rcs: xstrdup just wrappes strdup
Hi, thanks for the hint. This one should do the trick. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.9 diff -u -p -r1.9 xmalloc.c --- xmalloc.c 13 Jun 2015 20:15:21 - 1.9 +++ xmalloc.c 15 Jun 2015 07:52:15 - @@ -68,13 +68,10 @@ xreallocarray(void *ptr, size_t nmemb, s char * xstrdup(const char *str) { - size_t len; char *cp; - - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + + if ((cp = strdup(str)) == NULL) + err(1, xstrdup); return cp; }
Re: [patch]rcs: xstrdup just wrappes strdup
On Sun, Jun 14, 2015 at 05:02:05PM -0600, Theo de Raadt wrote: But I am not sure about this change. xmalloc.c came from ssh (and is also used by file and diff). Would it be better to keep it in sync? How portable is strdup? strdup is extremely portable. The last mainstream operating system which lacked it was Ultrix. So you could call it 100.00% portable. Since there is no portable version of openRCS is it that necessary to focus on portability? Functions like explicit_bzero(3) are not portable either, but used widely. Or is it more to find a balance between the usage of internal function, but make the tool not that hard to port to other systems? Regards, --F.
Re: [patch]rcs: xstrdup just wrappes strdup
On Wed, Jun 10, 2015 at 07:37:57PM +0200, Fritjof Bornebusch wrote: On Wed, May 20, 2015 at 10:55:34AM +0200, Fritjof Bornebusch wrote: On Tue, May 19, 2015 at 08:57:06PM +0200, Fritjof Bornebusch wrote: Hi, xstrdup just wrappes strdup, so there is no need to call xmalloc and strlcpy instead. Ping Without PGP stuff Use err() instead of errx(), so errno will be printed additionally. Thanks to Tim. Regards, --F. Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.8 diff -u -p -r1.8 xmalloc.c --- xmalloc.c 26 Mar 2015 15:17:30 - 1.8 +++ xmalloc.c 20 May 2015 08:53:01 - @@ -76,13 +76,11 @@ xfree(void *ptr) char * xstrdup(const char *str) { - size_t len; char *cp; - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + if ((cp = strdup(str)) == NULL) + err(1, xstrdup: copy string failed); + return cp; }
[patch]rcs: usage functions above the main ones
Hi tech@, most of the tools implements the *usage* function above the *main* function. This patch makes it more consistent to these tools and where the different *usage* functions are implemented in rcs in general. Any comments? Regards, --F. Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.121 diff -u -p -r1.121 co.c --- co.c13 Jun 2015 20:15:21 - 1.121 +++ co.c14 Jun 2015 20:21:41 - @@ -43,6 +43,17 @@ static void checkout_err_nobranch(RCSFIL const char *, int); static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *); +__dead void +checkout_usage(void) +{ + fprintf(stderr, + usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n + [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n + [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); + + exit(1); +} + int checkout_main(int argc, char **argv) { @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv) } return (ret); -} - -__dead void -checkout_usage(void) -{ - fprintf(stderr, - usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n - [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n - [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); - - exit(1); } /* Index: ident.c === RCS file: /cvs/src/usr.bin/rcs/ident.c,v retrieving revision 1.30 diff -u -p -r1.30 ident.c --- ident.c 2 Oct 2014 06:23:15 - 1.30 +++ ident.c 14 Jun 2015 20:21:41 - @@ -41,6 +41,14 @@ static int flags = 0; static voidident_file(const char *, FILE *); static voidident_line(FILE *); +__dead void +ident_usage(void) +{ + fprintf(stderr, usage: ident [-qV] [file ...]\n); + + exit(1); +} + int ident_main(int argc, char **argv) { @@ -158,12 +166,4 @@ ident_line(FILE *fp) out: if (bp != NULL) buf_free(bp); -} - -__dead void -ident_usage(void) -{ - fprintf(stderr, usage: ident [-qV] [file ...]\n); - - exit(1); } Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.9 diff -u -p -r1.9 merge.c --- merge.c 10 Oct 2014 08:15:25 - 1.9 +++ merge.c 14 Jun 2015 20:21:41 - @@ -32,6 +32,15 @@ #include rcsprog.h #include diff.h +__dead void +merge_usage(void) +{ + fprintf(stderr, + usage: merge [-EepqV] [-L label] file1 file2 file3\n); + + exit(D_ERROR); +} + int merge_main(int argc, char **argv) { @@ -108,13 +117,4 @@ merge_main(int argc, char **argv) buf_free(bp); return (status); -} - -__dead void -merge_usage(void) -{ - (void)fprintf(stderr, - usage: merge [-EepqV] [-L label] file1 file2 file3\n); - - exit(D_ERROR); } Index: rcsclean.c === RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v retrieving revision 1.54 diff -u -p -r1.54 rcsclean.c --- rcsclean.c 16 Jan 2015 06:40:11 - 1.54 +++ rcsclean.c 14 Jun 2015 20:21:41 - @@ -43,6 +43,16 @@ static int uflag = 0; static int flags = 0; static char *locker = NULL; +__dead void +rcsclean_usage(void) +{ + fprintf(stderr, + usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n + [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n); + + exit(1); +} + int rcsclean_main(int argc, char **argv) { @@ -116,16 +126,6 @@ rcsclean_main(int argc, char **argv) rcsclean_file(argv[i], rev_str); return (0); -} - -__dead void -rcsclean_usage(void) -{ - fprintf(stderr, - usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n - [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n); - - exit(1); } static void Index: rcsdiff.c === RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v retrieving revision 1.83 diff -u -p -r1.83 rcsdiff.c --- rcsdiff.c 13 Jun 2015 20:15:21 - 1.83 +++ rcsdiff.c 14 Jun 2015 20:21:41 - @@ -45,6 +45,16 @@ static int quiet; static int kflag = RCS_KWEXP_ERR; static char *diff_ignore_pats; +__dead void +rcsdiff_usage(void) +{ + fprintf(stderr, + usage: rcsdiff [-cnquV] [-kmode] [-rrev] [-xsuffixes] [-ztz]\n + [diff_options] file ...\n); + + exit(D_ERROR); +} + int rcsdiff_main(int argc, char **argv) { @@ -262,16 +272,6 @@ rcsdiff_main(int argc, char **argv) } return (status); -} - -__dead void -rcsdiff_usage(void) -{ - fprintf(stderr, - usage: rcsdiff [-cnquV] [-kmode] [-rrev] [-xsuffixes] [-ztz]\n -
Re: [patch]rcs: remove xfree
On Sat, Jun 13, 2015 at 09:33:59AM +0100, Nicholas Marriott wrote: Hi. You missed date.y: date.y: In function 'yyerror': date.y:497: error: implicit declaration of function 'xfree' Ups, sorry. That should do the trick. On Sat, Jun 13, 2015 at 12:43:29AM +0200, Fritjof Bornebusch wrote: Hi tech@, Without PGP / SMIME stuff, sorry. a couple of months ago I removed the if condition in the *xfree* function, but tedu@ suggested that it would be better to remove the *xfree* function entirely instead. If've seen there are *efree* functions in some tools, that just wrappes the free(3) function call. I'm not quite sure what is the best way todo it, so comments are very welcome. Regards, --F. Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.24 diff -u -p -r1.24 buf.c --- buf.c 5 Feb 2015 12:59:58 - 1.24 +++ buf.c 12 Jun 2015 22:20:32 - @@ -32,6 +32,7 @@ #include fcntl.h #include stdint.h #include stdio.h +#include stdlib.h #include string.h #include unistd.h @@ -137,15 +138,14 @@ out: void buf_free(BUF *b) { - if (b-cb_buf != NULL) - xfree(b-cb_buf); - xfree(b); + free(b-cb_buf); + free(b); } /* * Free the buffer b's structural information but do not free the contents * of the buffer. Instead, they are returned and should be freed later using - * xfree(). + * free(). */ void * buf_release(BUF *b) @@ -153,7 +153,7 @@ buf_release(BUF *b) void *tmp; tmp = b-cb_buf; - xfree(b); + free(b); return (tmp); } Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.219 diff -u -p -r1.219 ci.c --- ci.c16 Jan 2015 06:40:11 - 1.219 +++ ci.c12 Jun 2015 22:20:32 - @@ -211,7 +211,7 @@ checkin_main(int argc, char **argv) exit(0); case 'w': if (pb.author != NULL) - xfree(pb.author); + free(pb.author); pb.author = xstrdup(rcs_optarg); break; case 'x': @@ -376,10 +376,8 @@ out: buf_free(b2); if (b3 != NULL) buf_free(b3); - if (path1 != NULL) - xfree(path1); - if (path2 != NULL) - xfree(path2); + free(path1); + free(path2); return (NULL); } @@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb fprintf(stderr, reuse log message of previous file? [yn](y): ); if (rcs_yesno('y') != 'y') { - xfree(pb-rcs_msg); + free(pb-rcs_msg); pb-rcs_msg = NULL; } } @@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb pb-username, pb-author, NULL, NULL); if ((pb-flags INTERACTIVE) (pb-rcs_msg[0] == '\0')) { - xfree(pb-rcs_msg); /* free empty log message */ + free(pb-rcs_msg); /* free empty log message */ pb-rcs_msg = NULL; } @@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[3], tokens[4]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); if (i 6) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[5]); if (i 7) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[6]); break; case KW_TYPE_AUTHOR: if (i 2) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[1]); break; case KW_TYPE_DATE: @@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[1], tokens[2]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); break; case KW_TYPE_STATE: if (i 2) break; - if (*state != NULL) - xfree(*state
[patch]rcs: remove xfree
Hi tech@, Without PGP / SMIME stuff, sorry. a couple of months ago I removed the if condition in the *xfree* function, but tedu@ suggested that it would be better to remove the *xfree* function entirely instead. If've seen there are *efree* functions in some tools, that just wrappes the free(3) function call. I'm not quite sure what is the best way todo it, so comments are very welcome. Regards, --F. Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.24 diff -u -p -r1.24 buf.c --- buf.c 5 Feb 2015 12:59:58 - 1.24 +++ buf.c 12 Jun 2015 22:20:32 - @@ -32,6 +32,7 @@ #include fcntl.h #include stdint.h #include stdio.h +#include stdlib.h #include string.h #include unistd.h @@ -137,15 +138,14 @@ out: void buf_free(BUF *b) { - if (b-cb_buf != NULL) - xfree(b-cb_buf); - xfree(b); + free(b-cb_buf); + free(b); } /* * Free the buffer b's structural information but do not free the contents * of the buffer. Instead, they are returned and should be freed later using - * xfree(). + * free(). */ void * buf_release(BUF *b) @@ -153,7 +153,7 @@ buf_release(BUF *b) void *tmp; tmp = b-cb_buf; - xfree(b); + free(b); return (tmp); } Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.219 diff -u -p -r1.219 ci.c --- ci.c16 Jan 2015 06:40:11 - 1.219 +++ ci.c12 Jun 2015 22:20:32 - @@ -211,7 +211,7 @@ checkin_main(int argc, char **argv) exit(0); case 'w': if (pb.author != NULL) - xfree(pb.author); + free(pb.author); pb.author = xstrdup(rcs_optarg); break; case 'x': @@ -376,10 +376,8 @@ out: buf_free(b2); if (b3 != NULL) buf_free(b3); - if (path1 != NULL) - xfree(path1); - if (path2 != NULL) - xfree(path2); + free(path1); + free(path2); return (NULL); } @@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb fprintf(stderr, reuse log message of previous file? [yn](y): ); if (rcs_yesno('y') != 'y') { - xfree(pb-rcs_msg); + free(pb-rcs_msg); pb-rcs_msg = NULL; } } @@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb pb-username, pb-author, NULL, NULL); if ((pb-flags INTERACTIVE) (pb-rcs_msg[0] == '\0')) { - xfree(pb-rcs_msg); /* free empty log message */ + free(pb-rcs_msg); /* free empty log message */ pb-rcs_msg = NULL; } @@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[3], tokens[4]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); if (i 6) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[5]); if (i 7) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[6]); break; case KW_TYPE_AUTHOR: if (i 2) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[1]); break; case KW_TYPE_DATE: @@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[1], tokens[2]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); break; case KW_TYPE_STATE: if (i 2) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[1]); break; case KW_TYPE_REVISION: Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.120 diff -u -p -r1.120 co.c --- co.c16 Jan 2015 06:40:11 -
[patch]rcs: remove xfree
Hi tech@, a couple of months ago I removed the if condition in the *xfree* function, but tedu@ suggested that it would be better to remove the *xfree* function entirely instead. If've seen there are *efree* functions in some tools, that just wrappes the free(3) function call. I'm not quite sure what is the best way todo it, so comments are very welcome. Regards, --F. Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.24 diff -u -p -r1.24 buf.c --- buf.c 5 Feb 2015 12:59:58 - 1.24 +++ buf.c 12 Jun 2015 22:20:32 - @@ -32,6 +32,7 @@ #include fcntl.h #include stdint.h #include stdio.h +#include stdlib.h #include string.h #include unistd.h @@ -137,15 +138,14 @@ out: void buf_free(BUF *b) { - if (b-cb_buf != NULL) - xfree(b-cb_buf); - xfree(b); + free(b-cb_buf); + free(b); } /* * Free the buffer b's structural information but do not free the contents * of the buffer. Instead, they are returned and should be freed later using - * xfree(). + * free(). */ void * buf_release(BUF *b) @@ -153,7 +153,7 @@ buf_release(BUF *b) void *tmp; tmp = b-cb_buf; - xfree(b); + free(b); return (tmp); } Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.219 diff -u -p -r1.219 ci.c --- ci.c16 Jan 2015 06:40:11 - 1.219 +++ ci.c12 Jun 2015 22:20:32 - @@ -211,7 +211,7 @@ checkin_main(int argc, char **argv) exit(0); case 'w': if (pb.author != NULL) - xfree(pb.author); + free(pb.author); pb.author = xstrdup(rcs_optarg); break; case 'x': @@ -376,10 +376,8 @@ out: buf_free(b2); if (b3 != NULL) buf_free(b3); - if (path1 != NULL) - xfree(path1); - if (path2 != NULL) - xfree(path2); + free(path1); + free(path2); return (NULL); } @@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb fprintf(stderr, reuse log message of previous file? [yn](y): ); if (rcs_yesno('y') != 'y') { - xfree(pb-rcs_msg); + free(pb-rcs_msg); pb-rcs_msg = NULL; } } @@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb pb-username, pb-author, NULL, NULL); if ((pb-flags INTERACTIVE) (pb-rcs_msg[0] == '\0')) { - xfree(pb-rcs_msg); /* free empty log message */ + free(pb-rcs_msg); /* free empty log message */ pb-rcs_msg = NULL; } @@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[3], tokens[4]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); if (i 6) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[5]); if (i 7) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[6]); break; case KW_TYPE_AUTHOR: if (i 2) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[1]); break; case KW_TYPE_DATE: @@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[1], tokens[2]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); break; case KW_TYPE_STATE: if (i 2) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[1]); break; case KW_TYPE_REVISION: Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.120 diff -u -p -r1.120 co.c --- co.c16 Jan 2015 06:40:11 - 1.120 +++ co.c12 Jun 2015 22:20:32
Re: [patch]rcs: xstrdup just wrappes strdup
On Wed, May 20, 2015 at 10:55:34AM +0200, Fritjof Bornebusch wrote: On Tue, May 19, 2015 at 08:57:06PM +0200, Fritjof Bornebusch wrote: Hi, xstrdup just wrappes strdup, so there is no need to call xmalloc and strlcpy instead. Ping Use err() instead of errx(), so errno will be printed additionally. Thanks to Tim. Regards, --F. Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.8 diff -u -p -r1.8 xmalloc.c --- xmalloc.c 26 Mar 2015 15:17:30 - 1.8 +++ xmalloc.c 20 May 2015 08:53:01 - @@ -76,13 +76,11 @@ xfree(void *ptr) char * xstrdup(const char *str) { - size_t len; char *cp; - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + if ((cp = strdup(str)) == NULL) + err(1, xstrdup: copy string failed); + return cp; } pgp1q6Ck8BKwd.pgp Description: PGP signature
Re: [patch]rcs: xstrdup just wrappes strdup
On Tue, May 19, 2015 at 08:57:06PM +0200, Fritjof Bornebusch wrote: Hi, xstrdup just wrappes strdup, so there is no need to call xmalloc and strlcpy instead. Use err() instead of errx(), so errno will be printed additionally. Thanks to Tim. Regards, --F. Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.8 diff -u -p -r1.8 xmalloc.c --- xmalloc.c 26 Mar 2015 15:17:30 - 1.8 +++ xmalloc.c 20 May 2015 08:53:01 - @@ -76,13 +76,11 @@ xfree(void *ptr) char * xstrdup(const char *str) { - size_t len; char *cp; - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + if ((cp = strdup(str)) == NULL) + err(1, xstrdup: copy string failed); + return cp; } pgpW87xQmqoAq.pgp Description: PGP signature
Re: [patch]apmd ? sign
On Wed, May 20, 2015 at 09:35:03PM +0200, Alexander Hall wrote: On May 20, 2015 5:08:21 PM GMT+02:00, Fritjof Bornebusch frit...@alokat.org wrote: Hi, for what is the ? sign for? fallthrough to usage() But why is this necessary, haven't seen this in other deamons? BTW: isn't the \* FALLTHROUGH *\ comment missing? # apmd -? /Alexander --F. Regards, --F. Index: apmd.c === RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v retrieving revision 1.75 diff -u -p -r1.75 apmd.c --- apmd.c 6 Feb 2015 08:16:50 - 1.75 +++ apmd.c 20 May 2015 15:04:38 - @@ -403,7 +403,6 @@ main(int argc, char *argv[]) doperf = PERF_MANUAL; setperfpolicy(high); break; -case '?': default: usage(); } pgptou0PoyQly.pgp Description: PGP signature
[patch]apmd ? sign
Hi, for what is the ? sign for? Regards, --F. Index: apmd.c === RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v retrieving revision 1.75 diff -u -p -r1.75 apmd.c --- apmd.c 6 Feb 2015 08:16:50 - 1.75 +++ apmd.c 20 May 2015 15:04:38 - @@ -403,7 +403,6 @@ main(int argc, char *argv[]) doperf = PERF_MANUAL; setperfpolicy(high); break; - case '?': default: usage(); } pgp1bgAfQehas.pgp Description: PGP signature
[patch]rcs: xstrdup just wrappes strdup
Hi, xstrdup just wrappes strdup, so there is no need to call xmalloc and strlcpy instead. Regards, --F. Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.8 diff -u -p -r1.8 xmalloc.c --- xmalloc.c 26 Mar 2015 15:17:30 - 1.8 +++ xmalloc.c 19 May 2015 18:54:22 - @@ -76,13 +76,11 @@ xfree(void *ptr) char * xstrdup(const char *str) { - size_t len; char *cp; - len = strlen(str) + 1; - cp = xmalloc(len); - if (strlcpy(cp, str, len) = len) - errx(1, xstrdup: string truncated); + if ((cp = strdup(str)) == NULL) + errx(1, xstrdup: copy string failed); + return cp; } pgpsSRRSWzADK.pgp Description: PGP signature
Re: [patch]sudo: punctuation fixes
On Wed, Dec 24, 2014 at 01:48:44PM +0100, Fritjof Bornebusch wrote: Ping .. Hi tech@, looks like there are some missing periods regarding the sudo wrong password messages. fritjof Index: ins_csops.h === RCS file: /cvs/src/usr.bin/sudo/ins_csops.h,v retrieving revision 1.5 diff -u -p -r1.5 ins_csops.h --- ins_csops.h 4 Mar 2010 12:21:36 - 1.5 +++ ins_csops.h 24 Dec 2014 12:34:49 - @@ -34,6 +34,6 @@ #endif I've seen penguins that can type better than that., Have you considered trying to match wits with a rutabaga?, -You speak an infinite deal of nothing, +You speak an infinite deal of nothing., #endif /* _SUDO_INS_CSOPS_H */ Index: ins_classic.h === RCS file: /cvs/src/usr.bin/sudo/ins_classic.h,v retrieving revision 1.3 diff -u -p -r1.3 ins_classic.h --- ins_classic.h 4 Mar 2010 12:21:36 - 1.3 +++ ins_classic.h 24 Dec 2014 12:35:36 - @@ -21,7 +21,7 @@ * Insults from the original sudo(8). */ -Wrong! You cheating scum!, +Wrong! You cheating scum!, #ifdef PC_INSULTS And you call yourself a Rocket Scientist!, #else Index: ins_goons.h === RCS file: /cvs/src/usr.bin/sudo/ins_goons.h,v retrieving revision 1.3 diff -u -p -r1.3 ins_goons.h --- ins_goons.h 4 Mar 2010 12:21:36 - 1.3 +++ ins_goons.h 24 Dec 2014 12:38:56 - @@ -24,25 +24,25 @@ You silly, twisted boy you., He has fallen in the water!, We'll all be murdered in our beds!, -You can't come in. Our tiger has got flu, +You can't come in. Our tiger has got flu., I don't wish to know that., What, what, what, what, what, what, what, what, what, what?, You can't get the wood, you know., You'll starve!, ... and it used to be so popular..., -Pauses for audience applause, not a sausage, +Pauses for audience applause, not a sausage., Hold it up to the light --- not a brain in sight!, Have a gorilla..., There must be cure for it!, There's a lot of it about, you know., You do that again and see what happens..., -Ying Tong Iddle I Po, +Ying Tong Iddle I Po., Harm can come to a young lad like that!, And with that remarks folks, the case of the Crown vs yourself was proven., Speak English you fool --- there are no subtitles in this scene., You gotta go ow!, I have been called worse., It's only your word against mine., -I think ... err ... I think ... I think I'll go home, +I think ... err ... I think ... I think I'll go home., #endif /* _SUDO_INS_GOONS_H */ pgpYPOtIhUety.pgp Description: PGP signature
[patch] siphash static functions
Hi tech@, aren't these functions supposed to be static? fritjof Index: siphash.c === RCS file: /cvs/src/sys/crypto/siphash.c,v retrieving revision 1.1 diff -u -p -r1.1 siphash.c --- siphash.c 4 Nov 2014 03:01:14 - 1.1 +++ siphash.c 16 Jan 2015 10:41:37 - @@ -48,8 +48,8 @@ #include crypto/siphash.h -void SipHash_CRounds(SIPHASH_CTX *, int); -void SipHash_Rounds(SIPHASH_CTX *, int); +static void SipHash_CRounds(SIPHASH_CTX *, int); +static void SipHash_Rounds(SIPHASH_CTX *, int); void SipHash_Init(SIPHASH_CTX *ctx, const SIPHASH_KEY *key) @@ -147,7 +147,7 @@ SipHash(const SIPHASH_KEY *key, int rc, #define SIP_ROTL(x, b) ((x) (b)) | ( (x) (64 - (b))) -void +static void SipHash_Rounds(SIPHASH_CTX *ctx, int rounds) { while (rounds--) { @@ -171,7 +171,7 @@ SipHash_Rounds(SIPHASH_CTX *ctx, int rou } } -void +static void SipHash_CRounds(SIPHASH_CTX *ctx, int rounds) { u_int64_t m = lemtoh64((u_int64_t *)ctx-buf); pgpoz5I_1ymPA.pgp Description: PGP signature
[patch] remove atoi(3) from keynote
Hi tech@, this diff removes the atoi(3) call from keynote(1). fritjof Index: keynote-keygen.c === RCS file: /cvs/src/lib/libkeynote/keynote-keygen.c,v retrieving revision 1.21 diff -u -p -r1.21 keynote-keygen.c --- keynote-keygen.c29 Jun 2004 11:35:56 - 1.21 +++ keynote-keygen.c16 Jan 2015 19:44:42 - @@ -24,6 +24,7 @@ #include ctype.h #include fcntl.h +#include limits.h #include regex.h #include stdio.h #include stdlib.h @@ -106,6 +107,7 @@ keynote_keygen(int argc, char *argv[]) RSA *rsa; FILE *fp; char *algname; +const char *errstr; if ((argc != 5) (argc != 6) (argc != 7)) { @@ -135,8 +137,8 @@ keynote_keygen(int argc, char *argv[]) if (argc 5) { - begin = atoi(argv[5]); - if (begin = -1) + begin = strtonum(argv[5], 0, INT_MAX, errstr); + if (errstr) { fprintf(stderr, Erroneous value for print-offset parameter.\n); exit(1); @@ -145,8 +147,8 @@ keynote_keygen(int argc, char *argv[]) if (argc 6) { - prlen = atoi(argv[6]); - if (prlen = 0) + prlen = strtonum(argv[6], 1, INT_MAX, errstr); + if (errstr) { fprintf(stderr, Erroneous value for print-length parameter.\n); exit(1); @@ -162,9 +164,9 @@ keynote_keygen(int argc, char *argv[]) } alg = keynote_get_key_algorithm(algname, enc, ienc); -len = atoi(argv[2]); +len = strtonum(argv[2], 1, INT_MAX, errstr); -if (len = 0) +if (errstr) { fprintf(stderr, Invalid specified keysize %d\n, len); exit(1); Index: keynote-sign.c === RCS file: /cvs/src/lib/libkeynote/keynote-sign.c,v retrieving revision 1.16 diff -u -p -r1.16 keynote-sign.c --- keynote-sign.c 29 Jun 2004 11:35:56 - 1.16 +++ keynote-sign.c 16 Jan 2015 19:44:46 - @@ -23,6 +23,7 @@ #include sys/stat.h #include ctype.h +#include limits.h #include regex.h #include stdio.h #include stdlib.h @@ -50,6 +51,7 @@ keynote_sign(int argc, char *argv[]) char *buf, *buf2, *sig, *algname; int fd, flg = 0, buflen; struct stat sb; +const char *errstr; if ((argc != 4) (argc != 5) @@ -65,8 +67,8 @@ keynote_sign(int argc, char *argv[]) if (argc 4 + flg) { -begin = atoi(argv[4 + flg]); -if (begin = -1) + begin = strtonum(argv[4 + flg], 0, INT_MAX, errstr); +if (errstr) { fprintf(stderr, Erroneous value for print-offset parameter.\n); exit(1); @@ -75,8 +77,8 @@ keynote_sign(int argc, char *argv[]) if (argc 5 + flg) { -prlen = atoi(argv[5 + flg]); -if (prlen = 0) + prlen = strtonum(argv[5 + flg], 1, INT_MAX, errstr); +if (errstr) { fprintf(stderr, Erroneous value for print-length parameter.\n); exit(1); pgp__Xagg7l1q.pgp Description: PGP signature
[patch]sudo: punctuation fixes
Hi tech@, looks like there are some missing periods regarding the sudo wrong password messages. fritjof Index: ins_csops.h === RCS file: /cvs/src/usr.bin/sudo/ins_csops.h,v retrieving revision 1.5 diff -u -p -r1.5 ins_csops.h --- ins_csops.h 4 Mar 2010 12:21:36 - 1.5 +++ ins_csops.h 24 Dec 2014 12:34:49 - @@ -34,6 +34,6 @@ #endif I've seen penguins that can type better than that., Have you considered trying to match wits with a rutabaga?, -You speak an infinite deal of nothing, +You speak an infinite deal of nothing., #endif /* _SUDO_INS_CSOPS_H */ Index: ins_classic.h === RCS file: /cvs/src/usr.bin/sudo/ins_classic.h,v retrieving revision 1.3 diff -u -p -r1.3 ins_classic.h --- ins_classic.h 4 Mar 2010 12:21:36 - 1.3 +++ ins_classic.h 24 Dec 2014 12:35:36 - @@ -21,7 +21,7 @@ * Insults from the original sudo(8). */ -Wrong! You cheating scum!, +Wrong! You cheating scum!, #ifdef PC_INSULTS And you call yourself a Rocket Scientist!, #else Index: ins_goons.h === RCS file: /cvs/src/usr.bin/sudo/ins_goons.h,v retrieving revision 1.3 diff -u -p -r1.3 ins_goons.h --- ins_goons.h 4 Mar 2010 12:21:36 - 1.3 +++ ins_goons.h 24 Dec 2014 12:38:56 - @@ -24,25 +24,25 @@ You silly, twisted boy you., He has fallen in the water!, We'll all be murdered in our beds!, -You can't come in. Our tiger has got flu, +You can't come in. Our tiger has got flu., I don't wish to know that., What, what, what, what, what, what, what, what, what, what?, You can't get the wood, you know., You'll starve!, ... and it used to be so popular..., -Pauses for audience applause, not a sausage, +Pauses for audience applause, not a sausage., Hold it up to the light --- not a brain in sight!, Have a gorilla..., There must be cure for it!, There's a lot of it about, you know., You do that again and see what happens..., -Ying Tong Iddle I Po, +Ying Tong Iddle I Po., Harm can come to a young lad like that!, And with that remarks folks, the case of the Crown vs yourself was proven., Speak English you fool --- there are no subtitles in this scene., You gotta go ow!, I have been called worse., It's only your word against mine., -I think ... err ... I think ... I think I'll go home, +I think ... err ... I think ... I think I'll go home., #endif /* _SUDO_INS_GOONS_H */ pgppAPn70BBu8.pgp Description: PGP signature
[patch]rcs: correct error message after renaming realloc
fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.6 diff -u -p -r1.6 xmalloc.c --- xmalloc.c 1 Dec 2014 21:58:46 - 1.6 +++ xmalloc.c 1 Dec 2014 23:59:50 - @@ -60,7 +60,7 @@ xreallocarray(void *ptr, size_t nmemb, s new_ptr = reallocarray(ptr, nmemb, size); if (new_ptr == NULL) - errx(1, xrealloc: out of memory (new_size %zu bytes), + errx(1, xreallocarray: out of memory (new_size %zu bytes), nmemb * size); return new_ptr; } pgpWKr1YLtM1Z.pgp Description: PGP signature
[patch]rcs: comment typo
Hi tech, it's NULL not NUL. fritjof Index: diff3.c === RCS file: /cvs/src/usr.bin/rcs/diff3.c,v retrieving revision 1.33 diff -u -p -r1.33 diff3.c --- diff3.c 4 Mar 2012 04:05:15 - 1.33 +++ diff3.c 29 Nov 2014 13:15:51 - @@ -450,7 +450,7 @@ ed_patch_lines(struct rcs_lines *dlines, if (lp-l_len 2) continue; - /* NUL-terminate line buffer for strtol() safety. */ + /* NULL-terminate line buffer for strtol() safety. */ tmp = lp-l_line[lp-l_len - 1]; lp-l_line[lp-l_len - 1] = '\0'; Index: rcs.c === RCS file: /cvs/src/usr.bin/rcs/rcs.c,v retrieving revision 1.81 diff -u -p -r1.81 rcs.c --- rcs.c 10 Oct 2014 08:15:25 - 1.81 +++ rcs.c 29 Nov 2014 13:16:40 - @@ -799,7 +799,7 @@ rcs_patch_lines(struct rcs_lines *dlines if (lp-l_len 2) errx(1, line too short, RCS patch seems broken); op = *(lp-l_line); - /* NUL-terminate line buffer for strtol() safety. */ + /* NULL-terminate line buffer for strtol() safety. */ tmp = lp-l_line[lp-l_len - 1]; lp-l_line[lp-l_len - 1] = '\0'; lineno = (int)strtol((lp-l_line + 1), ep, 10); @@ -1047,7 +1047,7 @@ rcs_delta_stats(struct rcs_delta *rdp, i errx(1, line too short, RCS patch seems broken); op = *(lp-l_line); - /* NUL-terminate line buffer for strtol() safety. */ + /* NULL-terminate line buffer for strtol() safety. */ tmp = lp-l_line[lp-l_len - 1]; lp-l_line[lp-l_len - 1] = '\0'; (void)strtol((lp-l_line + 1), ep, 10); pgppOOEBjCeyo.pgp Description: PGP signature
Re: [patch]rcs: comment typo
On Sat, Nov 29, 2014 at 05:27:00AM -0800, Claus Assmann wrote: On Sat, Nov 29, 2014, Fritjof Bornebusch wrote: it's NULL not NUL. Not in this case... NULL: is a pointer (usually 0) NUL: is a character ('\0') Ahh I see, thank you. pgpMkIwf4S_cz.pgp Description: PGP signature
Re: [patch]rcs: comment typo
On Sat, Nov 29, 2014 at 04:53:28PM +0100, Otto Moerbeek wrote: On Sat, Nov 29, 2014 at 02:22:25PM +0100, Fritjof Bornebusch wrote: Hi tech, it's NULL not NUL. You're touching a big controversy here. Many developers say that NUL is the right term when rferring to chars and not pointers, And what is the correct term when referring to '0'? Or means '\0' and '0' the same. -Otto fritjof Index: diff3.c === RCS file: /cvs/src/usr.bin/rcs/diff3.c,v retrieving revision 1.33 diff -u -p -r1.33 diff3.c --- diff3.c 4 Mar 2012 04:05:15 - 1.33 +++ diff3.c 29 Nov 2014 13:15:51 - @@ -450,7 +450,7 @@ ed_patch_lines(struct rcs_lines *dlines, if (lp-l_len 2) continue; - /* NUL-terminate line buffer for strtol() safety. */ + /* NULL-terminate line buffer for strtol() safety. */ tmp = lp-l_line[lp-l_len - 1]; lp-l_line[lp-l_len - 1] = '\0'; Index: rcs.c === RCS file: /cvs/src/usr.bin/rcs/rcs.c,v retrieving revision 1.81 diff -u -p -r1.81 rcs.c --- rcs.c 10 Oct 2014 08:15:25 - 1.81 +++ rcs.c 29 Nov 2014 13:16:40 - @@ -799,7 +799,7 @@ rcs_patch_lines(struct rcs_lines *dlines if (lp-l_len 2) errx(1, line too short, RCS patch seems broken); op = *(lp-l_line); - /* NUL-terminate line buffer for strtol() safety. */ + /* NULL-terminate line buffer for strtol() safety. */ tmp = lp-l_line[lp-l_len - 1]; lp-l_line[lp-l_len - 1] = '\0'; lineno = (int)strtol((lp-l_line + 1), ep, 10); @@ -1047,7 +1047,7 @@ rcs_delta_stats(struct rcs_delta *rdp, i errx(1, line too short, RCS patch seems broken); op = *(lp-l_line); - /* NUL-terminate line buffer for strtol() safety. */ + /* NULL-terminate line buffer for strtol() safety. */ tmp = lp-l_line[lp-l_len - 1]; lp-l_line[lp-l_len - 1] = '\0'; (void)strtol((lp-l_line + 1), ep, 10); pgpxlyH4t3nOW.pgp Description: PGP signature
Re: [Patch]rcs: use rcsnum_cmp
On Fri, Nov 28, 2014 at 04:14:50PM +0100, Otto Moerbeek wrote: On Sun, Nov 23, 2014 at 05:19:16PM +0100, Fritjof Bornebusch wrote: Hi tech, like the XXX comment says, rcsnum_cmp() can be used instead of a *for* loop. The following shows the original behavior: $ co -r1.2 foo.txt,v foo.txt,v -- foo.txt revision 1.2 done $ co -r1.1 foo.txt,v foo.txt,v -- foo.txt revision 1.1 done $ co foo.txt,v foo.txt,v -- foo.txt revision 1.2 done The following shows the changed behavior: $ co -r1.2 foo.txt,v foo.txt,v -- foo.txt revision 1.2 done $ co -r1.1 foo.txt,v foo.txt,v -- foo.txt revision 1.1 done $ co foo.txt,v foo.txt,v -- foo.txt revision 1.2 done Could some verify that I didn't miss a test case. Hmm, your examples show no diffference. I think the intention is to not change the behaviour, right? Yes. But as Tobias mentioned the behaviour will change anyway using my diff. I'm checking out the official GnuRCS version regarding this right now. -Otto fritjof fritjof Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.119 diff -u -p -r1.119 co.c --- co.c10 Oct 2014 08:15:25 - 1.119 +++ co.c23 Nov 2014 15:40:30 - @@ -265,18 +265,14 @@ checkout_rev(RCSFILE *file, RCSNUM *frev (void)fprintf(stderr, no revisions present; generating empty revision 0.0\n); - /* XXX rcsnum_cmp() + /* * Check out the latest revision if frev is greater than HEAD */ if (file-rf_ndelta != 0) { - for (i = 0; i file-rf_head-rn_len; i++) { - if (file-rf_head-rn_id[i] frev-rn_id[i]) { - frev = file-rf_head; - break; - } - } + if (rcsnum_cmp(file-rf_head, frev, 0) == 1) + frev = file-rf_head; } - + lcount = 0; TAILQ_FOREACH(lkp, (file-rf_locks), rl_list) { if (!strcmp(lkp-rl_name, lockname)) Index: rcs.c === RCS file: /cvs/src/usr.bin/rcs/rcs.c,v retrieving revision 1.81 diff -u -p -r1.81 rcs.c --- rcs.c 10 Oct 2014 08:15:25 - 1.81 +++ rcs.c 23 Nov 2014 15:56:58 - @@ -905,16 +905,15 @@ rcs_getrev(RCSFILE *rfp, RCSNUM *frev) else rev = frev; - /* XXX rcsnum_cmp() */ - for (i = 0; i rfp-rf_head-rn_len; i++) { - if (rfp-rf_head-rn_id[i] rev-rn_id[i]) { - rcs_errno = RCS_ERR_NOENT; - return (NULL); - } + if(rcsnum_cmp(rfp-rf_head, rev, 0) == 1) { + rcs_errno = RCS_ERR_NOENT; + return (NULL); } - - /* No matter what, we'll need everything parsed up until the description - so go for it. */ + + /* +* No matter what, we'll need everything parsed up until the description +* so go for it. +*/ if (rcsparse_deltas(rfp, NULL)) return (NULL); pgpZD2UEfcOPl.pgp Description: PGP signature
[Patch]rcs: use rcsnum_cmp
Hi tech, like the XXX comment says, rcsnum_cmp() can be used instead of a *for* loop. The following shows the original behavior: $ co -r1.2 foo.txt,v foo.txt,v -- foo.txt revision 1.2 done $ co -r1.1 foo.txt,v foo.txt,v -- foo.txt revision 1.1 done $ co foo.txt,v foo.txt,v -- foo.txt revision 1.2 done The following shows the changed behavior: $ co -r1.2 foo.txt,v foo.txt,v -- foo.txt revision 1.2 done $ co -r1.1 foo.txt,v foo.txt,v -- foo.txt revision 1.1 done $ co foo.txt,v foo.txt,v -- foo.txt revision 1.2 done Could some verify that I didn't miss a test case. fritjof Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.119 diff -u -p -r1.119 co.c --- co.c10 Oct 2014 08:15:25 - 1.119 +++ co.c23 Nov 2014 15:40:30 - @@ -265,18 +265,14 @@ checkout_rev(RCSFILE *file, RCSNUM *frev (void)fprintf(stderr, no revisions present; generating empty revision 0.0\n); - /* XXX rcsnum_cmp() + /* * Check out the latest revision if frev is greater than HEAD */ if (file-rf_ndelta != 0) { - for (i = 0; i file-rf_head-rn_len; i++) { - if (file-rf_head-rn_id[i] frev-rn_id[i]) { - frev = file-rf_head; - break; - } - } + if (rcsnum_cmp(file-rf_head, frev, 0) == 1) + frev = file-rf_head; } - + lcount = 0; TAILQ_FOREACH(lkp, (file-rf_locks), rl_list) { if (!strcmp(lkp-rl_name, lockname)) Index: rcs.c === RCS file: /cvs/src/usr.bin/rcs/rcs.c,v retrieving revision 1.81 diff -u -p -r1.81 rcs.c --- rcs.c 10 Oct 2014 08:15:25 - 1.81 +++ rcs.c 23 Nov 2014 15:56:58 - @@ -905,16 +905,15 @@ rcs_getrev(RCSFILE *rfp, RCSNUM *frev) else rev = frev; - /* XXX rcsnum_cmp() */ - for (i = 0; i rfp-rf_head-rn_len; i++) { - if (rfp-rf_head-rn_id[i] rev-rn_id[i]) { - rcs_errno = RCS_ERR_NOENT; - return (NULL); - } + if(rcsnum_cmp(rfp-rf_head, rev, 0) == 1) { + rcs_errno = RCS_ERR_NOENT; + return (NULL); } - - /* No matter what, we'll need everything parsed up until the description - so go for it. */ + + /* +* No matter what, we'll need everything parsed up until the description +* so go for it. +*/ if (rcsparse_deltas(rfp, NULL)) return (NULL); pgp7P8LDyEFuk.pgp Description: PGP signature
[PATCH]rcs: write usage function pointer always the same way
Hi tech, I think it's more readable if the usage() function pointer will always be written the same way. fritjof Index: rlog.c === RCS file: /cvs/src/usr.bin/rcs/rlog.c,v retrieving revision 1.69 diff -u -p -r1.69 rlog.c --- rlog.c 10 Oct 2014 08:15:25 - 1.69 +++ rlog.c 20 Nov 2014 15:54:28 - @@ -136,7 +136,7 @@ rlog_main(int argc, char **argv) timezone_flag = rcs_optarg; break; default: - (usage()); + (usage)(); } } pgpnNScZfk7ra.pgp Description: PGP signature
[patch]rcs: memcmp against 0
Hi, it's better to compare memcmp against 0, for clarity. fritjof Index: diff3.c === RCS file: /cvs/src/usr.bin/rcs/diff3.c,v retrieving revision 1.33 diff -u -p -r1.33 diff3.c --- diff3.c 4 Mar 2012 04:05:15 - 1.33 +++ diff3.c 21 Jun 2014 21:03:30 - @@ -517,7 +517,7 @@ ed_patch_lines(struct rcs_lines *dlines, if (lp == NULL) errx(1, ed_patch_lines); - if (!memcmp(lp-l_line, ., 1)) + if (memcmp(lp-l_line, ., 1) == 0) break; TAILQ_REMOVE((plines-l_lines), lp, l_list);
Re: [patch]lock and unlock like GnuRCS
On Tue, Oct 07, 2014 at 03:10:44AM -0400, Daniel Dickman wrote: Fritjof, have you let the gnu rcs project know about the segfault? Maybe see how they choose to fix things and then follow their lead? No, I have not. I hope they follow the tech@ mailing list. :) On Mon, Oct 6, 2014 at 10:47 AM, Nicholas Marriott nicholas.marri...@gmail.com wrote: I think that GNU RCS segfaulting for -u -l is enough justification to do what we like, so a message (and last flag wins) like -L/-U would be fine with me. But if we want to do what they probably meant to happen then better to match -l -u like Fritjof's diff. On Fri, Oct 03, 2014 at 12:55:35PM +0200, Otto Moerbeek wrote: On Thu, Oct 02, 2014 at 12:56:10AM +0100, Nicholas Marriott wrote: OTOH, check out what we do with rcs -L and -U... I kinda like that, because it tells you exactly what it is doing. -Otto On Thu, Oct 02, 2014 at 12:54:13AM +0100, Nicholas Marriott wrote: Matching GNU RCS seems preferable to me but I don't feel strongly about it. I wouldn't mention this in the man page, it hardly seems like behaviour anyone should (or will need to) rely on. On Wed, Oct 01, 2014 at 07:41:52PM -0400, Daniel Dickman wrote: posix commands (like ls(1) for example) keep the last option when mutually exclusive options are specified. does it make sense to keep rcs consistent with that convention? also is a man page diff needed? On Oct 1, 2014, at 7:17 PM, Nicholas Marriott nicholas.marri...@gmail.com wrote: The existing behaviour isn't wildly useful, makes sense to me, ok nicm On Wed, Oct 01, 2014 at 11:33:33PM +0200, Fritjof Bornebusch wrote: Hi tech, the OpenRCS rcs command produces the following output if -l and -u is used in the same command: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked I've looked at GnuRCS and it has another way to handle these parameters (it seems the other BSDs use GnuRCS, too). Debian 7.5: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v rcs: foo.txt,v: no lock set on revision 1.1 1.1 locked $ rcs -u1.1 -l1.1 foo.txt Segmentation fault Well, I think the Segmentation fault isn't that important :), but GnuRCS does not lock and unlock a file by using the same command like OpenRCS. I think the different implementations of RCS should share the same behaviour: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked done $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 unlocked done fritjof Index: rcsprog.c === RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v retrieving revision 1.151 diff -u -p -r1.151 rcsprog.c --- rcsprog.c12 Jul 2011 21:00:32 -1.151 +++ rcsprog.c3 Aug 2014 15:42:34 - @@ -234,9 +234,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_STRICT; break; case 'l': -/* XXX - Check with -u flag. */ -lrev = rcs_optarg; -rcsflags |= RCSPROG_LFLAG; +if (!(rcsflags RCSPROG_UFLAG)) { +lrev = rcs_optarg; +rcsflags |= RCSPROG_LFLAG; +} break; case 'm': if (logstr != NULL) @@ -272,9 +273,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_LOOSE; break; case 'u': -/* XXX - Check with -l flag. */ -urev = rcs_optarg; -rcsflags |= RCSPROG_UFLAG; +if (!(rcsflags RCSPROG_LFLAG)) { +urev = rcs_optarg; +rcsflags |= RCSPROG_UFLAG; +} break; case 'V': printf(%s\n, rcs_version);
Re: [patch]lock and unlock like GnuRCS
On Tue, Oct 07, 2014 at 09:34:33AM +0200, Otto Moerbeek wrote: On Tue, Oct 07, 2014 at 03:10:44AM -0400, Daniel Dickman wrote: Fritjof, have you let the gnu rcs project know about the segfault? Maybe see how they choose to fix things and then follow their lead? That will only slow things down. Do what -L -U does is better, imo. Otto, so you appreciate a diff more like this one? -Otto fritjof Index: rcsprog.c === RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v retrieving revision 1.152 diff -u -p -r1.152 rcsprog.c --- rcsprog.c 2 Oct 2014 06:23:15 - 1.152 +++ rcsprog.c 7 Oct 2014 12:53:10 - @@ -235,9 +235,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_STRICT; break; case 'l': - /* XXX - Check with -u flag. */ + if (rcsflags RCSPROG_UFLAG) + warnx(-u overridden by -l); lrev = rcs_optarg; - rcsflags |= RCSPROG_LFLAG; + rcsflags = RCSPROG_LFLAG; break; case 'm': if (logstr != NULL) @@ -273,9 +274,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_LOOSE; break; case 'u': - /* XXX - Check with -l flag. */ + if (rcsflags RCSPROG_LFLAG) + warnx(-l overridden by -u); urev = rcs_optarg; - rcsflags |= RCSPROG_UFLAG; + rcsflags = RCSPROG_UFLAG; break; case 'V': printf(%s\n, rcs_version);
Re: [patch]lock and unlock like GnuRCS
On Tue, Oct 07, 2014 at 03:11:28PM +0200, Otto Moerbeek wrote: On Tue, Oct 07, 2014 at 02:56:07PM +0200, Fritjof Bornebusch wrote: On Tue, Oct 07, 2014 at 09:34:33AM +0200, Otto Moerbeek wrote: On Tue, Oct 07, 2014 at 03:10:44AM -0400, Daniel Dickman wrote: Fritjof, have you let the gnu rcs project know about the segfault? Maybe see how they choose to fix things and then follow their lead? That will only slow things down. Do what -L -U does is better, imo. Otto, so you appreciate a diff more like this one? well, almost. I think it should be clear one flag and set one, not clear all and set one. Here is the new one. -Otto fritjof fritjof Index: rcsprog.c === RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v retrieving revision 1.152 diff -u -p -r1.152 rcsprog.c --- rcsprog.c 2 Oct 2014 06:23:15 - 1.152 +++ rcsprog.c 7 Oct 2014 15:08:39 - @@ -235,8 +235,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_STRICT; break; case 'l': - /* XXX - Check with -u flag. */ + if (rcsflags RCSPROG_UFLAG) + warnx(-u overridden by -l); lrev = rcs_optarg; + rcsflags = ~RCSPROG_UFLAG; rcsflags |= RCSPROG_LFLAG; break; case 'm': @@ -273,8 +275,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_LOOSE; break; case 'u': - /* XXX - Check with -l flag. */ + if (rcsflags RCSPROG_LFLAG) + warnx(-l overridden by -u); urev = rcs_optarg; + rcsflags = ~RCSPROG_LFLAG; rcsflags |= RCSPROG_UFLAG; break; case 'V':
Re: [Patch] use exit() directly in usage()
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote: Hi, Hi, after usage() was called, there is no where you can go. as suggested by otto@ and @nicm, the usage() functions are marked as __dead. fritjof fritjof Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.217 diff -u -p -r1.217 ci.c --- ci.c19 May 2014 19:42:24 - 1.217 +++ ci.c1 Oct 2014 08:38:31 - @@ -89,7 +89,7 @@ static voidcheckin_parsekeyword(char * static int checkin_update(struct checkin_params *); static int checkin_revert(struct checkin_params *); -void +__dead void checkin_usage(void) { fprintf(stderr, @@ -97,6 +97,8 @@ checkin_usage(void) [-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n [-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n); + + exit(1); } /* @@ -221,7 +223,6 @@ checkin_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -231,7 +232,6 @@ checkin_main(int argc, char **argv) if (argc == 0) { warnx(no input file); (usage)(); - exit(1); } if ((pb.username = getlogin()) == NULL) Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.117 diff -u -p -r1.117 co.c --- co.c16 Apr 2013 20:24:45 - 1.117 +++ co.c1 Oct 2014 08:38:40 - @@ -79,7 +79,6 @@ checkout_main(int argc, char **argv) if (RCS_KWEXP_INVAL(kflag)) { warnx(invalid RCS keyword substitution mode); (usage)(); - exit(1); } break; case 'l': @@ -141,7 +140,6 @@ checkout_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -151,7 +149,6 @@ checkout_main(int argc, char **argv) if (argc == 0) { warnx(no input file); (usage)(); - exit (1); } if ((username = getlogin()) == NULL) @@ -222,13 +219,15 @@ checkout_main(int argc, char **argv) return (ret); } -void +__dead void checkout_usage(void) { fprintf(stderr, usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); + + exit(1); } /* Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.7 diff -u -p -r1.7 merge.c --- merge.c 23 Jul 2010 21:46:05 - 1.7 +++ merge.c 1 Oct 2014 08:38:52 - @@ -77,7 +77,6 @@ merge_main(int argc, char **argv) exit(0); default: (usage)(); - exit(D_ERROR); } } argc -= optind; @@ -86,7 +85,6 @@ merge_main(int argc, char **argv) if (argc != 3) { warnx(%s arguments, (argc 3) ? not enough : too many); (usage)(); - exit(D_ERROR); } for (; labels 3; labels++) @@ -113,9 +111,11 @@ merge_main(int argc, char **argv) return (status); } -void +__dead void merge_usage(void) { (void)fprintf(stderr, usage: merge [-EepqV] [-L label] file1 file2 file3\n); + + exit(D_ERROR); } Index: rcsclean.c === RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v retrieving revision 1.52 diff -u -p -r1.52 rcsclean.c --- rcsclean.c 28 Jul 2010 09:07:11 - 1.52 +++ rcsclean.c 1 Oct 2014 08:39:05 - @@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv) if (RCS_KWEXP_INVAL(kflag)) { warnx(invalid RCS keyword substitution mode); (usage)(); - exit(1); } break; case 'n': @@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -104,7 +102,6 @@ rcsclean_main(int argc, char **argv) if ((dirp = opendir(.)) == NULL) { warn(opendir); (usage
Re: [Patch] use exit() directly in usage()
On Wed, Oct 01, 2014 at 06:41:25PM +0100, Nicholas Marriott wrote: Looks good but you have missed out ident.c and rcsprog.c Ups, sorry. On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote: On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote: Hi, Hi, after usage() was called, there is no where you can go. as suggested by otto@ and @nicm, the usage() functions are marked as __dead. fritjof fritjof fritjof Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.217 diff -u -p -r1.217 ci.c --- ci.c19 May 2014 19:42:24 - 1.217 +++ ci.c1 Oct 2014 08:38:31 - @@ -89,7 +89,7 @@ static voidcheckin_parsekeyword(char * static int checkin_update(struct checkin_params *); static int checkin_revert(struct checkin_params *); -void +__dead void checkin_usage(void) { fprintf(stderr, @@ -97,6 +97,8 @@ checkin_usage(void) [-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n [-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n); + + exit(1); } /* @@ -221,7 +223,6 @@ checkin_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -231,7 +232,6 @@ checkin_main(int argc, char **argv) if (argc == 0) { warnx(no input file); (usage)(); - exit(1); } if ((pb.username = getlogin()) == NULL) Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.117 diff -u -p -r1.117 co.c --- co.c16 Apr 2013 20:24:45 - 1.117 +++ co.c1 Oct 2014 08:38:40 - @@ -79,7 +79,6 @@ checkout_main(int argc, char **argv) if (RCS_KWEXP_INVAL(kflag)) { warnx(invalid RCS keyword substitution mode); (usage)(); - exit(1); } break; case 'l': @@ -141,7 +140,6 @@ checkout_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -151,7 +149,6 @@ checkout_main(int argc, char **argv) if (argc == 0) { warnx(no input file); (usage)(); - exit (1); } if ((username = getlogin()) == NULL) @@ -222,13 +219,15 @@ checkout_main(int argc, char **argv) return (ret); } -void +__dead void checkout_usage(void) { fprintf(stderr, usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); + + exit(1); } /* Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.7 diff -u -p -r1.7 merge.c --- merge.c 23 Jul 2010 21:46:05 - 1.7 +++ merge.c 1 Oct 2014 08:38:52 - @@ -77,7 +77,6 @@ merge_main(int argc, char **argv) exit(0); default: (usage)(); - exit(D_ERROR); } } argc -= optind; @@ -86,7 +85,6 @@ merge_main(int argc, char **argv) if (argc != 3) { warnx(%s arguments, (argc 3) ? not enough : too many); (usage)(); - exit(D_ERROR); } for (; labels 3; labels++) @@ -113,9 +111,11 @@ merge_main(int argc, char **argv) return (status); } -void +__dead void merge_usage(void) { (void)fprintf(stderr, usage: merge [-EepqV] [-L label] file1 file2 file3\n); + + exit(D_ERROR); } Index: rcsclean.c === RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v retrieving revision 1.52 diff -u -p -r1.52 rcsclean.c --- rcsclean.c 28 Jul 2010 09:07:11 - 1.52 +++ rcsclean.c 1 Oct 2014 08:39:05 - @@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv) if (RCS_KWEXP_INVAL(kflag)) { warnx(invalid RCS keyword substitution mode); (usage)(); - exit(1); } break; case 'n': @@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv) break; default: (usage
[patch]lock and unlock like GnuRCS
Hi tech, the OpenRCS rcs command produces the following output if -l and -u is used in the same command: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked I've looked at GnuRCS and it has another way to handle these parameters (it seems the other BSDs use GnuRCS, too). Debian 7.5: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v rcs: foo.txt,v: no lock set on revision 1.1 1.1 locked $ rcs -u1.1 -l1.1 foo.txt Segmentation fault Well, I think the Segmentation fault isn't that important :), but GnuRCS does not lock and unlock a file by using the same command like OpenRCS. I think the different implementations of RCS should share the same behaviour: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked done $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 unlocked done fritjof Index: rcsprog.c === RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v retrieving revision 1.151 diff -u -p -r1.151 rcsprog.c --- rcsprog.c 12 Jul 2011 21:00:32 - 1.151 +++ rcsprog.c 3 Aug 2014 15:42:34 - @@ -234,9 +234,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_STRICT; break; case 'l': - /* XXX - Check with -u flag. */ - lrev = rcs_optarg; - rcsflags |= RCSPROG_LFLAG; + if (!(rcsflags RCSPROG_UFLAG)) { + lrev = rcs_optarg; + rcsflags |= RCSPROG_LFLAG; + } break; case 'm': if (logstr != NULL) @@ -272,9 +273,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_LOOSE; break; case 'u': - /* XXX - Check with -l flag. */ - urev = rcs_optarg; - rcsflags |= RCSPROG_UFLAG; + if (!(rcsflags RCSPROG_LFLAG)) { + urev = rcs_optarg; + rcsflags |= RCSPROG_UFLAG; + } break; case 'V': printf(%s\n, rcs_version);
[Patch] avoid typecast
Hi, there is no need for the typecast. fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 20 Jun 2014 10:21:37 - @@ -32,8 +32,8 @@ xmalloc(size_t size) ptr = malloc(size); if (ptr == NULL) errx(1, - xmalloc: out of memory (allocating %lu bytes), - (u_long) size); + xmalloc: out of memory (allocating %zu bytes), + size); return ptr; } @@ -48,8 +48,8 @@ xcalloc(size_t nmemb, size_t size) errx(1, xcalloc: nmemb * size SIZE_MAX); ptr = calloc(nmemb, size); if (ptr == NULL) - errx(1, xcalloc: out of memory (allocating %lu bytes), - (u_long)(size * nmemb)); + errx(1, xcalloc: out of memory (allocating %zu bytes), + (size * nmemb)); return ptr; } @@ -68,8 +68,8 @@ xrealloc(void *ptr, size_t nmemb, size_t else new_ptr = realloc(ptr, new_size); if (new_ptr == NULL) - errx(1, xrealloc: out of memory (new_size %lu bytes), - (u_long) new_size); + errx(1, xrealloc: out of memory (new_size %zu bytes), + new_size); return new_ptr; }
Re: [Patch]openrcs: atoi to strtonum
On Wed, Sep 24, 2014 at 10:31:17PM +0200, Otto Moerbeek wrote: Hi, On Wed, Sep 24, 2014 at 05:13:47PM +0200, Fritjof Bornebusch wrote: Hi, I changed atoi to strtonum in order to avoid overflows. One concern: atoi() does not mind trailing stuff, while strtonum() does. Did you verify that the strings are just numbers in all cases? according to the code and the manpages there are two different methods available to specify the timezone. - LT - +-hh:mm LT is handled seperatly and the code below - atoi(3) - only converts the hour and minute string values after seperation into int, e.g. +09:88 - h = 09; m = 88. The + or - sign will be handled in a different part of the code. I think this diff won't change functionality. -Otto fritjof fritjof Index: rcstime.c === RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v retrieving revision 1.4 diff -u -p -r1.4 rcstime.c --- rcstime.c 29 Apr 2014 07:44:19 - 1.4 +++ rcstime.c 24 Sep 2014 15:06:42 - @@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r int tzone; int pos; char *h, *m; + const char *errstr; struct tm *ltb; time_t now; @@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r memcpy(tb, rdp-rd_date, sizeof(*tb)); - tzone = atoi(h); - if ((tzone = 24) || (tzone = -24)) + tzone = strtonum(h, -23, 23, errstr); + if (errstr) errx(1, %s: not a known time zone, tz); if (pos) { @@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r tb-tm_hour = 0; if (m != NULL) { - tzone = atoi(m); - if (tzone = 60) - errx(1, %s: not a known time zone, tz); + tzone = strtonum(m, 0, 59, errstr); + if (errstr) + errx(1, %s: not a known minute, m); if ((tb-tm_min + tzone) = 60) { tb-tm_hour++;
[Patch]openrcs: atoi to strtonum
Hi, I changed atoi to strtonum in order to avoid overflows. fritjof Index: rcstime.c === RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v retrieving revision 1.4 diff -u -p -r1.4 rcstime.c --- rcstime.c 29 Apr 2014 07:44:19 - 1.4 +++ rcstime.c 24 Sep 2014 15:06:42 - @@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r int tzone; int pos; char *h, *m; + const char *errstr; struct tm *ltb; time_t now; @@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r memcpy(tb, rdp-rd_date, sizeof(*tb)); - tzone = atoi(h); - if ((tzone = 24) || (tzone = -24)) + tzone = strtonum(h, -23, 23, errstr); + if (errstr) errx(1, %s: not a known time zone, tz); if (pos) { @@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r tb-tm_hour = 0; if (m != NULL) { - tzone = atoi(m); - if (tzone = 60) - errx(1, %s: not a known time zone, tz); + tzone = strtonum(m, 0, 59, errstr); + if (errstr) + errx(1, %s: not a known minute, m); if ((tb-tm_min + tzone) = 60) { tb-tm_hour++;
Re: [PATCH] rcs: don't use lock and unlock in the same command
On Sun, Aug 03, 2014 at 06:00:45PM +0200, Fritjof Bornebusch wrote: Ping? Hi tech, the OpenRCS rcs command produces the following output if -l and -u is used in the same command: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked I've looked at GnuRCS and it has another way to handle these parameters (it seems the other BSDs use GnuRCS, too). Debian 7.5: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v rcs: foo.txt,v: no lock set on revision 1.1 1.1 locked $ rcs -u1.1 -l1.1 foo.txt Segmentation fault Well, I think the Segmentation fault isn't that important, but GnuRCS does not lock and unlock a file by using the same command like OpenRCS. I think the different implementations of RCS should share the same behaviour, so I changed OpenRCS: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked done $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 unlocked done fritjof Index: rcsprog.c === RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v retrieving revision 1.151 diff -u -p -r1.151 rcsprog.c --- rcsprog.c 12 Jul 2011 21:00:32 - 1.151 +++ rcsprog.c 3 Aug 2014 15:42:34 - @@ -234,9 +234,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_STRICT; break; case 'l': - /* XXX - Check with -u flag. */ - lrev = rcs_optarg; - rcsflags |= RCSPROG_LFLAG; + if (!(rcsflags RCSPROG_UFLAG)) { + lrev = rcs_optarg; + rcsflags |= RCSPROG_LFLAG; + } break; case 'm': if (logstr != NULL) @@ -272,9 +273,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_LOOSE; break; case 'u': - /* XXX - Check with -l flag. */ - urev = rcs_optarg; - rcsflags |= RCSPROG_UFLAG; + if (!(rcsflags RCSPROG_LFLAG)) { + urev = rcs_optarg; + rcsflags |= RCSPROG_UFLAG; + } break; case 'V': printf(%s\n, rcs_version);
Re: [PATCH]unnecessary typecast in rcs xmalloc
On Wed, Jul 30, 2014 at 10:23:00PM +0200, Fritjof Bornebusch wrote: Ping? Hi tech, there is an unnecessary typecast in xmalloc.c of rcs. fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 20 Jun 2014 10:21:37 - @@ -32,8 +32,8 @@ xmalloc(size_t size) ptr = malloc(size); if (ptr == NULL) errx(1, - xmalloc: out of memory (allocating %lu bytes), - (u_long) size); + xmalloc: out of memory (allocating %zu bytes), + size); return ptr; } @@ -48,8 +48,8 @@ xcalloc(size_t nmemb, size_t size) errx(1, xcalloc: nmemb * size SIZE_MAX); ptr = calloc(nmemb, size); if (ptr == NULL) - errx(1, xcalloc: out of memory (allocating %lu bytes), - (u_long)(size * nmemb)); + errx(1, xcalloc: out of memory (allocating %zu bytes), + (size * nmemb)); return ptr; } @@ -68,8 +68,8 @@ xrealloc(void *ptr, size_t nmemb, size_t else new_ptr = realloc(ptr, new_size); if (new_ptr == NULL) - errx(1, xrealloc: out of memory (new_size %lu bytes), - (u_long) new_size); + errx(1, xrealloc: out of memory (new_size %zu bytes), + new_size); return new_ptr; }
Re: [PATCH]unused NULL check before calling free
On Sat, Aug 02, 2014 at 10:35:43PM +0200, Fritjof Bornebusch wrote: Ping? On Fri, Aug 01, 2014 at 08:03:58AM -0400, Ted Unangst wrote: Half true. :) The behavior is intended. I don't really know why they care about freeing null, but the intention is clearly to check for it; otherwise they would just call free() in the first place. (actually, i think the rationale is that to assure portability, you need a strict version of free that matches the broken version found elsewhere or you will accidentally do something not portable. under that assumption, and assuming we don't care about those platforms, the correct fix is to delete xfree entirely.) The X option, though, to malloc.conf doesn't check NULL to free. It just makes malloc abort instead of returning null. I don't think it's a useful option either way. Don't use it. Based on Teds suggestion, this diff deletes xfree() entirely. fritjof Index: xmalloc.h === RCS file: /cvs/src/usr.bin/rcs/xmalloc.h,v retrieving revision 1.1 diff -u -p -r1.1 xmalloc.h --- xmalloc.h 26 Apr 2006 02:55:13 - 1.1 +++ xmalloc.h 2 Aug 2014 20:09:55 - @@ -22,7 +22,6 @@ void *xmalloc(size_t); void *xcalloc(size_t, size_t); void *xrealloc(void *, size_t, size_t); -void xfree(void *); char *xstrdup(const char *); int xasprintf(char **, const char *, ...) __attribute__((__format__ (printf, 2, 3))) Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 2 Aug 2014 20:10:08 - @@ -73,14 +73,6 @@ xrealloc(void *ptr, size_t nmemb, size_t return new_ptr; } -void -xfree(void *ptr) -{ - if (ptr == NULL) - errx(1, xfree: NULL pointer given as argument); - free(ptr); -} - char * xstrdup(const char *str) { Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.217 diff -u -p -r1.217 ci.c --- ci.c 19 May 2014 19:42:24 - 1.217 +++ ci.c 2 Aug 2014 20:12:30 - @@ -208,8 +208,7 @@ checkin_main(int argc, char **argv) printf(%s\n, rcs_version); exit(0); case 'w': - if (pb.author != NULL) - xfree(pb.author); + free(pb.author); pb.author = xstrdup(rcs_optarg); break; case 'x': @@ -376,10 +375,9 @@ out: buf_free(b2); if (b3 != NULL) buf_free(b3); - if (path1 != NULL) - xfree(path1); - if (path2 != NULL) - xfree(path2); + + free(path1); + free(path2); return (NULL); } @@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb fprintf(stderr, reuse log message of previous file? [yn](y): ); if (rcs_yesno('y') != 'y') { - xfree(pb-rcs_msg); + free(pb-rcs_msg); pb-rcs_msg = NULL; } } @@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb pb-username, pb-author, NULL, NULL); if ((pb-flags INTERACTIVE) (pb-rcs_msg[0] == '\0')) { - xfree(pb-rcs_msg); /* free empty log message */ + free(pb-rcs_msg); /* free empty log message */ pb-rcs_msg = NULL; } @@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[3], tokens[4]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); if (i 6) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[5]); if (i 7) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[6]); break; case KW_TYPE_AUTHOR: if (i 2) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[1]); break; case KW_TYPE_DATE: @@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[1], tokens[2
Re: [PATCH] Better overflow handling in rcstime.c
On Wed, Jul 30, 2014 at 10:19:19PM +0200, Fritjof Bornebusch wrote: Ping? Hi tech, remove the atoi calls, in order to avoid overflows. fritjof Index: rcstime.c === RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v retrieving revision 1.4 diff -u -p -r1.4 rcstime.c --- rcstime.c 29 Apr 2014 07:44:19 - 1.4 +++ rcstime.c 30 Jun 2014 12:59:42 - @@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r int tzone; int pos; char *h, *m; + const char *errstr = NULL; struct tm *ltb; time_t now; @@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r memcpy(tb, rdp-rd_date, sizeof(*tb)); - tzone = atoi(h); - if ((tzone = 24) || (tzone = -24)) + tzone = (int)strtonum(h, -23, 23, errstr); + if (errstr) errx(1, %s: not a known time zone, tz); if (pos) { @@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r tb-tm_hour = 0; if (m != NULL) { - tzone = atoi(m); - if (tzone = 60) - errx(1, %s: not a known time zone, tz); + tzone = (int)strtonum(m, 0, 59, errstr); + if (errstr) + errx(1, %s: not a known minute, m); if ((tb-tm_min + tzone) = 60) { tb-tm_hour++;
Re: [PATCH]delete xfree() from sndiod
On Sun, Aug 03, 2014 at 02:56:25PM +0200, Fritjof Bornebusch wrote: Ping? Hi tech, during my search after other xfree() implementations, I saw that xfree() in sndiod is just a wrapper for free() without any other conditions, like NULL check. fritjof Index: abuf.c === RCS file: /cvs/src/usr.bin/sndiod/abuf.c,v retrieving revision 1.2 diff -u -p -r1.2 abuf.c --- abuf.c7 Dec 2012 08:04:58 - 1.2 +++ abuf.c3 Aug 2014 12:37:19 - @@ -62,7 +62,7 @@ abuf_done(struct abuf *buf) } } #endif - xfree(buf-data); + free(buf-data); buf-data = (void *)0xdeadbeef; } Index: dev.c === RCS file: /cvs/src/usr.bin/sndiod/dev.c,v retrieving revision 1.17 diff -u -p -r1.17 dev.c --- dev.c 2 Jun 2014 07:51:25 - 1.17 +++ dev.c 3 Aug 2014 12:38:52 - @@ -15,6 +15,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ #include stdio.h +#include stdlib.h #include string.h #include abuf.h @@ -838,10 +839,8 @@ dev_cycle(struct dev *d) */ s-pstate = SLOT_INIT; abuf_done(s-mix.buf); - if (s-mix.decbuf) - xfree(s-mix.decbuf); - if (s-mix.resampbuf) - xfree(s-mix.resampbuf); + free(s-mix.decbuf); + free(s-mix.resampbuf); s-ops-eof(s-arg); *ps = s-next; dev_mix_adjvol(d); @@ -1143,14 +1142,12 @@ dev_close(struct dev *d) d-slot_list = NULL; dev_sio_close(d); if (d-mode MODE_PLAY) { - if (d-encbuf != NULL) - xfree(d-encbuf); - xfree(d-pbuf); + free(d-encbuf); + free(d-pbuf); } if (d-mode MODE_REC) { - if (d-decbuf != NULL) - xfree(d-decbuf); - xfree(d-rbuf); + free(d-decbuf); + free(d-rbuf); } } @@ -1256,7 +1253,7 @@ dev_del(struct dev *d) } midi_del(d-midi); *p = d-next; - xfree(d); + free(d); } unsigned int @@ -1829,16 +1826,12 @@ slot_detach(struct slot *s) } *ps = s-next; if (s-mode MODE_RECMASK) { - if (s-sub.encbuf) - xfree(s-sub.encbuf); - if (s-sub.resampbuf) - xfree(s-sub.resampbuf); + free(s-sub.encbuf); + free(s-sub.resampbuf); } if (s-mode MODE_PLAY) { - if (s-mix.decbuf) - xfree(s-mix.decbuf); - if (s-mix.resampbuf) - xfree(s-mix.resampbuf); + free(s-mix.decbuf); + free(s-mix.resampbuf); dev_mix_adjvol(s-dev); } } Index: file.c === RCS file: /cvs/src/usr.bin/sndiod/file.c,v retrieving revision 1.5 diff -u -p -r1.5 file.c --- file.c17 Mar 2014 17:17:01 - 1.5 +++ file.c3 Aug 2014 12:40:30 - @@ -294,7 +294,7 @@ file_poll(void) while ((f = *pf) != NULL) { if (f-state == FILE_ZOMB) { *pf = f-next; - xfree(f); + free(f); } else pf = f-next; } Index: listen.c === RCS file: /cvs/src/usr.bin/sndiod/listen.c,v retrieving revision 1.2 diff -u -p -r1.2 listen.c --- listen.c 13 Mar 2013 08:28:33 - 1.2 +++ listen.c 3 Aug 2014 12:40:55 - @@ -70,13 +70,13 @@ listen_close(struct listen *f) } *pf = f-next; - if (f-path != NULL) { + if (f-path != NULL) unlink(f-path); - xfree(f-path); - } + + free(f-path); file_del(f-file); close(f-fd); - xfree(f); + free(f); } void Index: midi.c === RCS file: /cvs/src/usr.bin/sndiod/midi.c,v retrieving revision 1.10 diff -u -p -r1.10 midi.c --- midi.c28 Sep 2013 18:49:32 - 1.10 +++ midi.c3 Aug 2014 12:42:08 - @@ -461,7 +461,7 @@ port_del(struct port *c) #endif } *p = c-next; - xfree(c); + free(c); } int Index: opt.c === RCS file: /cvs/src/usr.bin/sndiod/opt.c,v retrieving revision 1.2 diff -u -p -r1.2 opt.c --- opt.c 7 Dec 2012 08:04:58 - 1.2 +++ opt.c 3 Aug 2014 12:43:52 - @@ -136,5 +136,5 @@ opt_del(struct opt *o) #endif } *po = o
[PATCH]delete xfree() from sndiod
Hi tech, during my search after other xfree() implementations, I saw that xfree() in sndiod is just a wrapper for free() without any other conditions, like NULL check. fritjof Index: abuf.c === RCS file: /cvs/src/usr.bin/sndiod/abuf.c,v retrieving revision 1.2 diff -u -p -r1.2 abuf.c --- abuf.c 7 Dec 2012 08:04:58 - 1.2 +++ abuf.c 3 Aug 2014 12:37:19 - @@ -62,7 +62,7 @@ abuf_done(struct abuf *buf) } } #endif - xfree(buf-data); + free(buf-data); buf-data = (void *)0xdeadbeef; } Index: dev.c === RCS file: /cvs/src/usr.bin/sndiod/dev.c,v retrieving revision 1.17 diff -u -p -r1.17 dev.c --- dev.c 2 Jun 2014 07:51:25 - 1.17 +++ dev.c 3 Aug 2014 12:38:52 - @@ -15,6 +15,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ #include stdio.h +#include stdlib.h #include string.h #include abuf.h @@ -838,10 +839,8 @@ dev_cycle(struct dev *d) */ s-pstate = SLOT_INIT; abuf_done(s-mix.buf); - if (s-mix.decbuf) - xfree(s-mix.decbuf); - if (s-mix.resampbuf) - xfree(s-mix.resampbuf); + free(s-mix.decbuf); + free(s-mix.resampbuf); s-ops-eof(s-arg); *ps = s-next; dev_mix_adjvol(d); @@ -1143,14 +1142,12 @@ dev_close(struct dev *d) d-slot_list = NULL; dev_sio_close(d); if (d-mode MODE_PLAY) { - if (d-encbuf != NULL) - xfree(d-encbuf); - xfree(d-pbuf); + free(d-encbuf); + free(d-pbuf); } if (d-mode MODE_REC) { - if (d-decbuf != NULL) - xfree(d-decbuf); - xfree(d-rbuf); + free(d-decbuf); + free(d-rbuf); } } @@ -1256,7 +1253,7 @@ dev_del(struct dev *d) } midi_del(d-midi); *p = d-next; - xfree(d); + free(d); } unsigned int @@ -1829,16 +1826,12 @@ slot_detach(struct slot *s) } *ps = s-next; if (s-mode MODE_RECMASK) { - if (s-sub.encbuf) - xfree(s-sub.encbuf); - if (s-sub.resampbuf) - xfree(s-sub.resampbuf); + free(s-sub.encbuf); + free(s-sub.resampbuf); } if (s-mode MODE_PLAY) { - if (s-mix.decbuf) - xfree(s-mix.decbuf); - if (s-mix.resampbuf) - xfree(s-mix.resampbuf); + free(s-mix.decbuf); + free(s-mix.resampbuf); dev_mix_adjvol(s-dev); } } Index: file.c === RCS file: /cvs/src/usr.bin/sndiod/file.c,v retrieving revision 1.5 diff -u -p -r1.5 file.c --- file.c 17 Mar 2014 17:17:01 - 1.5 +++ file.c 3 Aug 2014 12:40:30 - @@ -294,7 +294,7 @@ file_poll(void) while ((f = *pf) != NULL) { if (f-state == FILE_ZOMB) { *pf = f-next; - xfree(f); + free(f); } else pf = f-next; } Index: listen.c === RCS file: /cvs/src/usr.bin/sndiod/listen.c,v retrieving revision 1.2 diff -u -p -r1.2 listen.c --- listen.c13 Mar 2013 08:28:33 - 1.2 +++ listen.c3 Aug 2014 12:40:55 - @@ -70,13 +70,13 @@ listen_close(struct listen *f) } *pf = f-next; - if (f-path != NULL) { + if (f-path != NULL) unlink(f-path); - xfree(f-path); - } + + free(f-path); file_del(f-file); close(f-fd); - xfree(f); + free(f); } void Index: midi.c === RCS file: /cvs/src/usr.bin/sndiod/midi.c,v retrieving revision 1.10 diff -u -p -r1.10 midi.c --- midi.c 28 Sep 2013 18:49:32 - 1.10 +++ midi.c 3 Aug 2014 12:42:08 - @@ -461,7 +461,7 @@ port_del(struct port *c) #endif } *p = c-next; - xfree(c); + free(c); } int Index: opt.c === RCS file: /cvs/src/usr.bin/sndiod/opt.c,v retrieving revision 1.2 diff -u -p -r1.2 opt.c --- opt.c 7 Dec 2012 08:04:58 - 1.2 +++ opt.c 3 Aug 2014 12:43:52 - @@ -136,5 +136,5 @@ opt_del(struct opt *o) #endif } *po = o-next; - xfree(o); + free(o); } Index:
[PATCH] rcs: don't use lock and unlock in the same command
Hi tech, the OpenRCS rcs command produces the following output if -l and -u is used in the same command: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 locked 1.1 unlocked I've looked at GnuRCS and it has another way to handle these parameters (it seems the other BSDs use GnuRCS, too). Debian 7.5: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v rcs: foo.txt,v: no lock set on revision 1.1 1.1 locked $ rcs -u1.1 -l1.1 foo.txt Segmentation fault Well, I think the Segmentation fault isn't that important, but GnuRCS does not lock and unlock a file by using the same command like OpenRCS. I think the different implementations of RCS should share the same behaviour, so I changed OpenRCS: $ rcs -l1.1 -u1.1 foo.txt RCS file: foo.txt,v 1.1 locked done $ rcs -u1.1 -l1.1 foo.txt RCS file: foo.txt,v 1.1 unlocked done fritjof Index: rcsprog.c === RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v retrieving revision 1.151 diff -u -p -r1.151 rcsprog.c --- rcsprog.c 12 Jul 2011 21:00:32 - 1.151 +++ rcsprog.c 3 Aug 2014 15:42:34 - @@ -234,9 +234,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_STRICT; break; case 'l': - /* XXX - Check with -u flag. */ - lrev = rcs_optarg; - rcsflags |= RCSPROG_LFLAG; + if (!(rcsflags RCSPROG_UFLAG)) { + lrev = rcs_optarg; + rcsflags |= RCSPROG_LFLAG; + } break; case 'm': if (logstr != NULL) @@ -272,9 +273,10 @@ rcs_main(int argc, char **argv) lkmode = RCS_LOCK_LOOSE; break; case 'u': - /* XXX - Check with -l flag. */ - urev = rcs_optarg; - rcsflags |= RCSPROG_UFLAG; + if (!(rcsflags RCSPROG_LFLAG)) { + urev = rcs_optarg; + rcsflags |= RCSPROG_UFLAG; + } break; case 'V': printf(%s\n, rcs_version);
Re: [PATCH]unused NULL check before calling free
On Fri, Aug 01, 2014 at 08:03:58AM -0400, Ted Unangst wrote: Half true. :) The behavior is intended. I don't really know why they care about freeing null, but the intention is clearly to check for it; otherwise they would just call free() in the first place. (actually, i think the rationale is that to assure portability, you need a strict version of free that matches the broken version found elsewhere or you will accidentally do something not portable. under that assumption, and assuming we don't care about those platforms, the correct fix is to delete xfree entirely.) The X option, though, to malloc.conf doesn't check NULL to free. It just makes malloc abort instead of returning null. I don't think it's a useful option either way. Don't use it. Based on Teds suggestion, this diff deletes xfree() entirely. fritjof Index: xmalloc.h === RCS file: /cvs/src/usr.bin/rcs/xmalloc.h,v retrieving revision 1.1 diff -u -p -r1.1 xmalloc.h --- xmalloc.h 26 Apr 2006 02:55:13 - 1.1 +++ xmalloc.h 2 Aug 2014 20:09:55 - @@ -22,7 +22,6 @@ void *xmalloc(size_t); void *xcalloc(size_t, size_t); void *xrealloc(void *, size_t, size_t); -void xfree(void *); char *xstrdup(const char *); int xasprintf(char **, const char *, ...) __attribute__((__format__ (printf, 2, 3))) Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 2 Aug 2014 20:10:08 - @@ -73,14 +73,6 @@ xrealloc(void *ptr, size_t nmemb, size_t return new_ptr; } -void -xfree(void *ptr) -{ - if (ptr == NULL) - errx(1, xfree: NULL pointer given as argument); - free(ptr); -} - char * xstrdup(const char *str) { Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.217 diff -u -p -r1.217 ci.c --- ci.c19 May 2014 19:42:24 - 1.217 +++ ci.c2 Aug 2014 20:12:30 - @@ -208,8 +208,7 @@ checkin_main(int argc, char **argv) printf(%s\n, rcs_version); exit(0); case 'w': - if (pb.author != NULL) - xfree(pb.author); + free(pb.author); pb.author = xstrdup(rcs_optarg); break; case 'x': @@ -376,10 +375,9 @@ out: buf_free(b2); if (b3 != NULL) buf_free(b3); - if (path1 != NULL) - xfree(path1); - if (path2 != NULL) - xfree(path2); + + free(path1); + free(path2); return (NULL); } @@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb fprintf(stderr, reuse log message of previous file? [yn](y): ); if (rcs_yesno('y') != 'y') { - xfree(pb-rcs_msg); + free(pb-rcs_msg); pb-rcs_msg = NULL; } } @@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb pb-username, pb-author, NULL, NULL); if ((pb-flags INTERACTIVE) (pb-rcs_msg[0] == '\0')) { - xfree(pb-rcs_msg); /* free empty log message */ + free(pb-rcs_msg); /* free empty log message */ pb-rcs_msg = NULL; } @@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[3], tokens[4]); if ((*date = date_parse(datestring)) == -1) errx(1, could not parse date); - xfree(datestring); + free(datestring); if (i 6) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[5]); if (i 7) break; - if (*state != NULL) - xfree(*state); + free(*state); *state = xstrdup(tokens[6]); break; case KW_TYPE_AUTHOR: if (i 2) break; - if (*author != NULL) - xfree(*author); + free(*author); *author = xstrdup(tokens[1]); break; case KW_TYPE_DATE: @@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC (void)xasprintf(datestring, %s %s, tokens[1], tokens[2]); if ((*date = date_parse(datestring)) == -1)
Re: [PATCH] Better overflow handling in rcstime.c
On Wed, Jul 30, 2014 at 09:26:54PM +0100, Dimitris Papastamos wrote: On Wed, Jul 30, 2014 at 10:19:19PM +0200, Fritjof Bornebusch wrote: + tzone = (int)strtonum(h, -23, 23, errstr); The explicit cast is not needed here. That's maybe true, but I don't like implicit casts. :) fritjof
Re: [PATCH]unused NULL check before calling free
On Wed, Jul 30, 2014 at 07:37:29PM -0700, patrick keshishian wrote: On Wed, Jul 30, 2014 at 10:14:54PM +0200, Fritjof Bornebusch wrote: Hi tech, there is an unnecessary NULL check before calling free. fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 31 May 2014 19:19:18 - @@ -76,8 +76,6 @@ xrealloc(void *ptr, size_t nmemb, size_t void xfree(void *ptr) { - if (ptr == NULL) - errx(1, xfree: NULL pointer given as argument); free(ptr); } Going by this context, a quick grep in src/usr.bin/rcs, and especially the error message, the NULL check's purpose is to catach this condition. The code you claim to correct: there is an unnecessary NULL check before calling free. would have been of the form: if (ptr != NULL)-or-if (ptr == NULL) free(ptr); return; That is true, but free() can handle NULL itself: ... /* This is legal. */ if (ptr == NULL) return; That's why there is no need, to use a condition like: if(ptr == NULL) errx(1, xfree: NULL pointer given as argument); --patrick fritjof
Re: [PATCH]unused NULL check before calling free
On Thu, Jul 31, 2014 at 10:32:07AM -0400, sven falempin wrote: On Thu, Jul 31, 2014 at 6:39 AM, Fritjof Bornebusch frit...@alokat.org wrote: On Wed, Jul 30, 2014 at 07:37:29PM -0700, patrick keshishian wrote: On Wed, Jul 30, 2014 at 10:14:54PM +0200, Fritjof Bornebusch wrote: Hi tech, there is an unnecessary NULL check before calling free. fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 31 May 2014 19:19:18 - @@ -76,8 +76,6 @@ xrealloc(void *ptr, size_t nmemb, size_t void xfree(void *ptr) { - if (ptr == NULL) - errx(1, xfree: NULL pointer given as argument); free(ptr); } Going by this context, a quick grep in src/usr.bin/rcs, and especially the error message, the NULL check's purpose is to catach this condition. The code you claim to correct: there is an unnecessary NULL check before calling free. would have been of the form: if (ptr != NULL)-or-if (ptr == NULL) free(ptr); return; That is true, but free() can handle NULL itself: ... /* This is legal. */ if (ptr == NULL) return; That's why there is no need, to use a condition like: if(ptr == NULL) errx(1, xfree: NULL pointer given as argument); --patrick fritjof For educational purpose i guess humanity need a free(NULL) is valid T shirt, i learned it in the mailing list around a year ago. Maybe the back would be if not, patch your OS, or better use OpenBSD That would be very cool! -- - () ascii ribbon campaign - against html e-mail /\
[PATCH]unused NULL check before calling free
Hi tech, there is an unnecessary NULL check before calling free. fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 31 May 2014 19:19:18 - @@ -76,8 +76,6 @@ xrealloc(void *ptr, size_t nmemb, size_t void xfree(void *ptr) { - if (ptr == NULL) - errx(1, xfree: NULL pointer given as argument); free(ptr); }
[PATCH] Better overflow handling in rcstime.c
Hi tech, remove the atoi calls, in order to avoid overflows. fritjof Index: rcstime.c === RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v retrieving revision 1.4 diff -u -p -r1.4 rcstime.c --- rcstime.c 29 Apr 2014 07:44:19 - 1.4 +++ rcstime.c 30 Jun 2014 12:59:42 - @@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r int tzone; int pos; char *h, *m; + const char *errstr = NULL; struct tm *ltb; time_t now; @@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r memcpy(tb, rdp-rd_date, sizeof(*tb)); - tzone = atoi(h); - if ((tzone = 24) || (tzone = -24)) + tzone = (int)strtonum(h, -23, 23, errstr); + if (errstr) errx(1, %s: not a known time zone, tz); if (pos) { @@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r tb-tm_hour = 0; if (m != NULL) { - tzone = atoi(m); - if (tzone = 60) - errx(1, %s: not a known time zone, tz); + tzone = (int)strtonum(m, 0, 59, errstr); + if (errstr) + errx(1, %s: not a known minute, m); if ((tb-tm_min + tzone) = 60) { tb-tm_hour++;
[PATCH]unnecessary typecast in rcs xmalloc
Hi tech, there is an unnecessary typecast in xmalloc.c of rcs. fritjof Index: xmalloc.c === RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v retrieving revision 1.4 diff -u -p -r1.4 xmalloc.c --- xmalloc.c 7 Jun 2009 08:39:13 - 1.4 +++ xmalloc.c 20 Jun 2014 10:21:37 - @@ -32,8 +32,8 @@ xmalloc(size_t size) ptr = malloc(size); if (ptr == NULL) errx(1, - xmalloc: out of memory (allocating %lu bytes), - (u_long) size); + xmalloc: out of memory (allocating %zu bytes), + size); return ptr; } @@ -48,8 +48,8 @@ xcalloc(size_t nmemb, size_t size) errx(1, xcalloc: nmemb * size SIZE_MAX); ptr = calloc(nmemb, size); if (ptr == NULL) - errx(1, xcalloc: out of memory (allocating %lu bytes), - (u_long)(size * nmemb)); + errx(1, xcalloc: out of memory (allocating %zu bytes), + (size * nmemb)); return ptr; } @@ -68,8 +68,8 @@ xrealloc(void *ptr, size_t nmemb, size_t else new_ptr = realloc(ptr, new_size); if (new_ptr == NULL) - errx(1, xrealloc: out of memory (new_size %lu bytes), - (u_long) new_size); + errx(1, xrealloc: out of memory (new_size %zu bytes), + new_size); return new_ptr; }
Re: [PATCH]unnecessary return in arc4random
Am I wrong? On Thu, May 22, 2014 at 04:30:03PM +0200, Fritjof Bornebusch wrote: Hi tech, does this return makes any sense, because it's a void function and the return is at the end of the function. fritjof Index: arc4random.c === RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v retrieving revision 1.30 diff -u -p -r1.30 arc4random.c --- arc4random.c6 May 2014 16:06:33 - 1.30 +++ arc4random.c22 May 2014 14:21:35 - @@ -165,7 +165,6 @@ _rs_random_u32(u_int32_t *val) memcpy(val, rs_buf + RSBUFSZ - rs_have, sizeof(*val)); memset(rs_buf + RSBUFSZ - rs_have, 0, sizeof(*val)); rs_have -= sizeof(*val); - return; } u_int32_t
[PATCH]unnecessary return in arc4random
Hi tech, does this return makes any sense, because it's a void function and the return is at the end of the function. fritjof Index: arc4random.c === RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v retrieving revision 1.30 diff -u -p -r1.30 arc4random.c --- arc4random.c6 May 2014 16:06:33 - 1.30 +++ arc4random.c22 May 2014 14:21:35 - @@ -165,7 +165,6 @@ _rs_random_u32(u_int32_t *val) memcpy(val, rs_buf + RSBUFSZ - rs_have, sizeof(*val)); memset(rs_buf + RSBUFSZ - rs_have, 0, sizeof(*val)); rs_have -= sizeof(*val); - return; } u_int32_t
[PATCH] rcs regression tests
Hi tech, I added some missing ; to the rlog out files, to make sure these tests don't fail. fritjof Index: rlog-rflag2.out === RCS file: /cvs/src/regress/usr.bin/rcs/rlog-rflag2.out,v retrieving revision 1.1 diff -u -p -r1.1 rlog-rflag2.out --- rlog-rflag2.out 20 Apr 2006 17:17:22 - 1.1 +++ rlog-rflag2.out 14 May 2014 22:01:55 - @@ -12,11 +12,11 @@ description: descr revision 1.3 -date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0 +date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0; third rev revision 1.2 -date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0 +date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0; second rev revision 1.1 Index: rlog-rflag3.out === RCS file: /cvs/src/regress/usr.bin/rcs/rlog-rflag3.out,v retrieving revision 1.1 diff -u -p -r1.1 rlog-rflag3.out --- rlog-rflag3.out 20 Apr 2006 17:17:22 - 1.1 +++ rlog-rflag3.out 14 May 2014 22:02:02 - @@ -12,10 +12,10 @@ description: descr revision 1.3 -date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0 +date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0; third rev revision 1.2 -date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0 +date: 2006/01/01 00:00:00; author: foo; state: Exp; lines: +1 -0; second rev =
[PATCH] rcs: free pointer
Hi tech, if ci uses a user defined revision number the pointer was just set to NULL and not freed correctly. fritjof Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.216 diff -u -p -r1.216 ci.c --- ci.c27 Oct 2013 18:31:24 - 1.216 +++ ci.c10 May 2014 15:30:51 - @@ -287,7 +287,6 @@ checkin_main(int argc, char **argv) (void)fprintf(stderr, %s -- %s\n, pb.fpath, pb.filename); - /* XXX - Should we rcsnum_free(pb.newrev)? */ if (rev_str != NULL) if ((pb.newrev = rcs_getrevnum(rev_str, pb.file)) == NULL) @@ -315,6 +314,8 @@ checkin_main(int argc, char **argv) } rcs_close(pb.file); + if (rev_str != NULL) + rcsnum_free(pb.newrev); pb.newrev = NULL; }
Re: [PATCH] comparison between signed and unsigned in rcs
On Wed, May 07, 2014 at 08:59:03PM +0200, Fritjof Bornebusch wrote: On Wed, May 07, 2014 at 08:05:35PM +0200, J??r??mie Courr??ges-Anglas wrote: Fritjof Bornebusch frit...@alokat.org writes: [...] Does no one want to check the diff and give me some feedback? Regardless of the content of your diff, the date of your mail was: Date: Tue, 6 May 2014 22:57:57 +0200 I doubt there are developers that are paid full-time to improve rcs, please be more patient. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE Sorry, you are right. Next time I'll be more patient. fritjof Come on, nobody? If my diffs point to the wrong direction, then please tell me! fritjof
Re: [PATCH] comparison between signed and unsigned in rcs
On Fri, May 09, 2014 at 12:35:11PM -0400, Kenneth Westerback wrote: On 9 May 2014 11:47, Kenneth Westerback kwesterb...@gmail.com wrote: On 9 May 2014 11:41, Fritjof Bornebusch frit...@alokat.org wrote: On Wed, May 07, 2014 at 08:59:03PM +0200, Fritjof Bornebusch wrote: On Wed, May 07, 2014 at 08:05:35PM +0200, J??r??mie Courr??ges-Anglas wrote: Fritjof Bornebusch frit...@alokat.org writes: [...] Does no one want to check the diff and give me some feedback? Regardless of the content of your diff, the date of your mail was: Date: Tue, 6 May 2014 22:57:57 +0200 I doubt there are developers that are paid full-time to improve rcs, please be more patient. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE Sorry, you are right. Next time I'll be more patient. fritjof Come on, nobody? If my diffs point to the wrong direction, then please tell me! fritjof No one has the time or interest apparently. Whining about it every couple of days does not help motivate people to drop their current work to look at your stylistic tweaks. Ken I apologize for the unintentionally harsh tone. We are always interested in getting diffs. Ken No problem. But if you like OpenRCS as it is and don't want to get any diffs unless security fixes, then just say it. Or if you work on other stuff and you don't have time for that, then just say it. fritjof
Re: [PATCH] comparison between signed and unsigned in rcs
On Fri, May 09, 2014 at 06:01:52PM +0200, J??r??mie Courr??ges-Anglas wrote: Fritjof Bornebusch frit...@alokat.org writes: Hi tech, Hi, if I compile rcs, gcc prints a few warnings like this: - comparison between signed and unsigned - signed and unsigned type in conditional expression I'm not quite sure if the typecasts are at the correct place, but these diffs removes the warnings. fritjof Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.22 diff -u -p -r1.22 buf.c --- buf.c 6 Jul 2011 15:36:52 - 1.22 +++ buf.c 6 May 2014 20:56:55 - @@ -98,7 +98,7 @@ buf_load(const char *path) if (fstat(fd, st) == -1) goto out; - if (st.st_size SIZE_MAX) { + if (st.st_size (off_t)SIZE_MAX) { errno = EFBIG; goto out; } This would break on 64 bits archs: there, SIZE_MAX casted to an off_t is -1 (implementation-defined behavior iirc). Also it breaks platforms with 32 bits off_t (this does not affect OpenBSD). I think the code is perfectly fine as is, but a possible clarification that does not do assumptions about the site of off_t and size_t could be: if ((uintmax_t)st.st_size (uintmax_t)SIZE_MAX) ... Index: diff.c === RCS file: /cvs/src/usr.bin/rcs/diff.c,v retrieving revision 1.34 diff -u -p -r1.34 diff.c --- diff.c 16 May 2013 12:44:48 - 1.34 +++ diff.c 6 May 2014 20:57:07 - @@ -432,13 +432,13 @@ prepare(int i, FILE *fd, off_t filesize, rewind(fd); - sz = (filesize = SIZE_MAX ? filesize : SIZE_MAX) / 25; + sz = (filesize = (off_t)SIZE_MAX ? filesize : (off_t)SIZE_MAX) / 25; if (sz 100) sz = 100; Same problem here. p = xcalloc(sz + 3, sizeof(*p)); for (j = 0; (h = readhash(fd, flags));) { - if (j == sz) { + if ((size_t)j == sz) { sz = sz * 3 / 2; p = xrealloc(p, sz + 3, sizeof(*p)); } Declaring j as a size_t would save a cast. Index: diff3.c === RCS file: /cvs/src/usr.bin/rcs/diff3.c,v retrieving revision 1.33 diff -u -p -r1.33 diff3.c --- diff3.c 4 Mar 2012 04:05:15 - 1.33 +++ diff3.c 6 May 2014 20:57:18 - @@ -908,7 +908,7 @@ edscript(int n) (void)fseek(fp[2], (long)de[n].new.from, SEEK_SET); for (k = de[n].new.to-de[n].new.from; k 0; k-= j) { j = k BUFSIZ ? BUFSIZ : k; - if (fread(block, 1, j, fp[2]) != j) + if ((int)fread(block, 1, j, fp[2]) != j) return (-1); block[j] = '\0'; diff_output(%s, block); Same here. Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.22 diff -u -p -r1.22 buf.c --- buf.c 6 Jul 2011 15:36:52 - 1.22 +++ buf.c 6 May 2014 20:57:30 - @@ -98,7 +98,7 @@ buf_load(const char *path) if (fstat(fd, st) == -1) goto out; - if (st.st_size SIZE_MAX) { + if (st.st_size (off_t)SIZE_MAX) { errno = EFBIG; goto out; } Duplicated hunk? -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE Hi, thank you for your feedback. I'll add it. fritjof
Re: [PATCH] rcs merge
On Thu, May 08, 2014 at 09:45:22AM +0200, Janne Johansson wrote: Can't say if this was the motivation here, but some people like to put constants before variables for comparisons so as to easily catch the difference between if (a = 5) ... and if (5 = a) .. when you really meant if (a == 5). You are right, some people like to see constants before variables for comparisions. But if you look later in the code you see something like if (argc != 3) or for (; labels 3; labels++). As you can see the variables are the first parameter for comparisions. So this diff makes it more consistent what format is used, too. fritjof 2014-05-08 0:13 GMT+02:00 Fritjof Bornebusch frit...@alokat.org: Hi tech, I think labels = 3 is more readable than 3 = labels. fritjof Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.7 diff -u -p -r1.7 merge.c --- merge.c 23 Jul 2010 21:46:05 - 1.7 +++ merge.c 7 May 2014 22:10:06 - @@ -62,7 +62,7 @@ merge_main(int argc, char **argv) flags |= MERGE_EFLAG; break; case 'L': - if (3 = labels) + if (labels = 3) errx(D_ERROR, too many -L options); label[labels++] = optarg; break; -- May the most significant bit of your life be positive.
[PATCH] rcs: no way to go, after usage was called
Hi tech, there is no way you can go, after usage() was called, so dont't do it. fritjof Index: ci.c === RCS file: /cvs/src/usr.bin/rcs/ci.c,v retrieving revision 1.216 diff -u -p -r1.216 ci.c --- ci.c27 Oct 2013 18:31:24 - 1.216 +++ ci.c8 May 2014 19:46:13 - @@ -97,6 +97,8 @@ checkin_usage(void) [-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n [-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n); + + exit(1); } /* @@ -221,7 +223,6 @@ checkin_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -231,7 +232,6 @@ checkin_main(int argc, char **argv) if (argc == 0) { warnx(no input file); (usage)(); - exit(1); } if ((pb.username = getlogin()) == NULL) Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.117 diff -u -p -r1.117 co.c --- co.c16 Apr 2013 20:24:45 - 1.117 +++ co.c8 May 2014 19:57:22 - @@ -79,7 +79,6 @@ checkout_main(int argc, char **argv) if (RCS_KWEXP_INVAL(kflag)) { warnx(invalid RCS keyword substitution mode); (usage)(); - exit(1); } break; case 'l': @@ -141,7 +140,6 @@ checkout_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -151,7 +149,6 @@ checkout_main(int argc, char **argv) if (argc == 0) { warnx(no input file); (usage)(); - exit (1); } if ((username = getlogin()) == NULL) @@ -229,6 +226,8 @@ checkout_usage(void) usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n); + + exit(1); } /* Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.7 diff -u -p -r1.7 merge.c --- merge.c 23 Jul 2010 21:46:05 - 1.7 +++ merge.c 8 May 2014 20:04:11 - @@ -77,7 +77,6 @@ merge_main(int argc, char **argv) exit(0); default: (usage)(); - exit(D_ERROR); } } argc -= optind; @@ -86,7 +85,6 @@ merge_main(int argc, char **argv) if (argc != 3) { warnx(%s arguments, (argc 3) ? not enough : too many); (usage)(); - exit(D_ERROR); } for (; labels 3; labels++) @@ -118,4 +116,6 @@ merge_usage(void) { (void)fprintf(stderr, usage: merge [-EepqV] [-L label] file1 file2 file3\n); + + exit(D_ERROR); } Index: rcsclean.c === RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v retrieving revision 1.52 diff -u -p -r1.52 rcsclean.c --- rcsclean.c 28 Jul 2010 09:07:11 - 1.52 +++ rcsclean.c 8 May 2014 20:05:52 - @@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv) if (RCS_KWEXP_INVAL(kflag)) { warnx(invalid RCS keyword substitution mode); (usage)(); - exit(1); } break; case 'n': @@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv) break; default: (usage)(); - exit(1); } } @@ -104,7 +102,6 @@ rcsclean_main(int argc, char **argv) if ((dirp = opendir(.)) == NULL) { warn(opendir); (usage)(); - exit(1); } while ((dp = readdir(dirp)) != NULL) { @@ -127,6 +124,8 @@ rcsclean_usage(void) fprintf(stderr, usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n); + + exit(1); } static void Index: rcsdiff.c === RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v retrieving revision 1.79 diff -u -p -r1.79 rcsdiff.c --- rcsdiff.c 16 Apr 2013 20:24:45 - 1.79 +++
Re: [PATCH] comparison between signed and unsigned in rcs
On Tue, May 06, 2014 at 10:57:57PM +0200, Fritjof Bornebusch wrote: Hi tech, if I compile rcs, gcc prints a few warnings like this: - comparison between signed and unsigned - signed and unsigned type in conditional expression I'm not quite sure if the typecasts are at the correct place, but these diffs removes the warnings. fritjof Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.22 diff -u -p -r1.22 buf.c --- buf.c 6 Jul 2011 15:36:52 - 1.22 +++ buf.c 6 May 2014 20:56:55 - @@ -98,7 +98,7 @@ buf_load(const char *path) if (fstat(fd, st) == -1) goto out; - if (st.st_size SIZE_MAX) { + if (st.st_size (off_t)SIZE_MAX) { errno = EFBIG; goto out; } Index: diff.c === RCS file: /cvs/src/usr.bin/rcs/diff.c,v retrieving revision 1.34 diff -u -p -r1.34 diff.c --- diff.c 16 May 2013 12:44:48 - 1.34 +++ diff.c 6 May 2014 20:57:07 - @@ -432,13 +432,13 @@ prepare(int i, FILE *fd, off_t filesize, rewind(fd); - sz = (filesize = SIZE_MAX ? filesize : SIZE_MAX) / 25; + sz = (filesize = (off_t)SIZE_MAX ? filesize : (off_t)SIZE_MAX) / 25; if (sz 100) sz = 100; p = xcalloc(sz + 3, sizeof(*p)); for (j = 0; (h = readhash(fd, flags));) { - if (j == sz) { + if ((size_t)j == sz) { sz = sz * 3 / 2; p = xrealloc(p, sz + 3, sizeof(*p)); } Index: diff3.c === RCS file: /cvs/src/usr.bin/rcs/diff3.c,v retrieving revision 1.33 diff -u -p -r1.33 diff3.c --- diff3.c 4 Mar 2012 04:05:15 - 1.33 +++ diff3.c 6 May 2014 20:57:18 - @@ -908,7 +908,7 @@ edscript(int n) (void)fseek(fp[2], (long)de[n].new.from, SEEK_SET); for (k = de[n].new.to-de[n].new.from; k 0; k-= j) { j = k BUFSIZ ? BUFSIZ : k; - if (fread(block, 1, j, fp[2]) != j) + if ((int)fread(block, 1, j, fp[2]) != j) return (-1); block[j] = '\0'; diff_output(%s, block); Index: buf.c === RCS file: /cvs/src/usr.bin/rcs/buf.c,v retrieving revision 1.22 diff -u -p -r1.22 buf.c --- buf.c 6 Jul 2011 15:36:52 - 1.22 +++ buf.c 6 May 2014 20:57:30 - @@ -98,7 +98,7 @@ buf_load(const char *path) if (fstat(fd, st) == -1) goto out; - if (st.st_size SIZE_MAX) { + if (st.st_size (off_t)SIZE_MAX) { errno = EFBIG; goto out; } Does no one want to check the diff and give me some feedback? fritjof
Re: [PATCH] comparison between signed and unsigned in rcs
On Wed, May 07, 2014 at 08:05:35PM +0200, J??r??mie Courr??ges-Anglas wrote: Fritjof Bornebusch frit...@alokat.org writes: [...] Does no one want to check the diff and give me some feedback? Regardless of the content of your diff, the date of your mail was: Date: Tue, 6 May 2014 22:57:57 +0200 I doubt there are developers that are paid full-time to improve rcs, please be more patient. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE Sorry, you are right. Next time I'll be more patient. fritjof
[PATCH] rcs void casts
Hi tech, there are a few void casts in rcs. But I have a question about that. Are these casts really necessary? I've read that the compiler warns, because of unused variables. But no compiler warnings about that on amd64. That's why I just added this small diff, in order to get feedback if the casts are necessary or not. fritjof Index: rlog.c === RCS file: /cvs/src/usr.bin/rcs/rlog.c,v retrieving revision 1.67 diff -u -p -r1.67 rlog.c --- rlog.c 7 Jan 2014 14:08:16 - 1.67 +++ rlog.c 7 May 2014 20:24:37 - @@ -517,7 +517,7 @@ rlog_rev_print(struct rcs_delta *rdp) fmt = %Y/%m/%d %H:%M:%S; } - (void)strftime(timeb, sizeof(timeb), fmt, t); + strftime(timeb, sizeof(timeb), fmt, t); printf(\ndate: %s; author: %s; state: %s;, timeb, rdp-rd_author, rdp-rd_state); @@ -556,7 +556,7 @@ rlog_rev_print(struct rcs_delta *rdp) TAILQ_FOREACH(rb, (rdp-rd_branches), rb_list) { RCSNUM *branch; branch = rcsnum_revtobr(rb-rb_num); - (void)rcsnum_tostr(branch, numb, sizeof(numb)); + rcsnum_tostr(branch, numb, sizeof(numb)); printf( %s;, numb); rcsnum_free(branch); }
Re: [PATCH] rcs void casts
On Wed, May 07, 2014 at 10:58:03PM +0200, Ingo Schwarze wrote: Hi Fritjof, Fritjof Bornebusch wrote on Wed, May 07, 2014 at 10:32:05PM +0200: there are a few void casts in rcs. But I have a question about that. Are these casts really necessary? No, they are not necessary. I've read that the compiler warns, because of unused variables. But no compiler warnings about that on amd64. Usually, you don't choose your style to please static analysis tools - there are too many of them, of too varying quality and signal-to-noise ratio, to please them all. Rather, it makes sense to tune static analysis tools, including compilers, to warn about dangerous style, but to not warn about style that is merely a matter of taste. That said, for functions that usually require checking of return values, the void cast can be used to indicate this call was audited, and ignoring the return value is considered safe, either because this specific call cannot actually fail, or because failure does not need explicit handling at this point. Typical functions where this makes sense are snprintf(3), strlcpy(3), strlcat(3). So i clearly wouldn't remove the cast from strftime() unless you have reason to assume that no proper audit was done, or this is broken and needs fixing. In this case, the buffer is large enough, so the cast should stay. On the other hand, on functions that hardly ever require checking the return code, casting to void is pointless, so i tend to remove such casts when editing nearby code. A typical example is (void)close(fd); which is rarely more helpful than just close(fd);. In this case, the rcsnum_tostr() return value can't be used for error checking in the first place, that function calls errx(3) on failure, so when editing nearby code, i'd probably remove that cast. But it doesn't warrant a commit on it's own imho. Yours, Ingo I see, thank you for the explanations. It was just for me to make it clear when (void) is used. fritjof Index: rlog.c === RCS file: /cvs/src/usr.bin/rcs/rlog.c,v retrieving revision 1.67 diff -u -p -r1.67 rlog.c --- rlog.c 7 Jan 2014 14:08:16 - 1.67 +++ rlog.c 7 May 2014 20:24:37 - @@ -517,7 +517,7 @@ rlog_rev_print(struct rcs_delta *rdp) fmt = %Y/%m/%d %H:%M:%S; } - (void)strftime(timeb, sizeof(timeb), fmt, t); + strftime(timeb, sizeof(timeb), fmt, t); printf(\ndate: %s; author: %s; state: %s;, timeb, rdp-rd_author, rdp-rd_state); @@ -556,7 +556,7 @@ rlog_rev_print(struct rcs_delta *rdp) TAILQ_FOREACH(rb, (rdp-rd_branches), rb_list) { RCSNUM *branch; branch = rcsnum_revtobr(rb-rb_num); - (void)rcsnum_tostr(branch, numb, sizeof(numb)); + rcsnum_tostr(branch, numb, sizeof(numb)); printf( %s;, numb); rcsnum_free(branch); }
[PATCH] rcs merge
Hi tech, I think labels = 3 is more readable than 3 = labels. fritjof Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.7 diff -u -p -r1.7 merge.c --- merge.c 23 Jul 2010 21:46:05 - 1.7 +++ merge.c 7 May 2014 22:10:06 - @@ -62,7 +62,7 @@ merge_main(int argc, char **argv) flags |= MERGE_EFLAG; break; case 'L': - if (3 = labels) + if (labels = 3) errx(D_ERROR, too many -L options); label[labels++] = optarg; break;
[PATCH] rcs stored values never read
Hi tech, there are some never read values in rcs. fritjof Index: co.c === RCS file: /cvs/src/usr.bin/rcs/co.c,v retrieving revision 1.117 diff -u -p -r1.117 co.c --- co.c16 Apr 2013 20:24:45 - 1.117 +++ co.c6 May 2014 19:57:15 - @@ -56,7 +56,6 @@ checkout_main(int argc, char **argv) flags = ret = 0; kflag = RCS_KWEXP_ERR; - rev = RCS_HEAD_REV; rev_str = NULL; author = date = state = NULL; @@ -257,7 +256,7 @@ checkout_rev(RCSFILE *file, RCSNUM *frev time_t rcsdate, givendate; RCSNUM *rev; - rcsdate = givendate = -1; + givendate = -1; if (date != NULL (givendate = date_parse(date)) == -1) { warnx(invalid date: %s, date); return -1; Index: diff.c === RCS file: /cvs/src/usr.bin/rcs/diff.c,v retrieving revision 1.34 diff -u -p -r1.34 diff.c --- diff.c 16 May 2013 12:44:48 - 1.34 +++ diff.c 6 May 2014 20:00:43 - @@ -1302,7 +1302,7 @@ dump_unified_vec(FILE *f1, FILE *f2, int if (context_vec_start context_vec_ptr) return; - b = d = 0; /* gcc */ + d = 0; /* gcc */ lowa = MAX(1, cvp-a - diff_context); upb = MIN(len[0], context_vec_ptr-b + diff_context); lowc = MAX(1, cvp-c - diff_context); Index: merge.c === RCS file: /cvs/src/usr.bin/rcs/merge.c,v retrieving revision 1.7 diff -u -p -r1.7 merge.c --- merge.c 23 Jul 2010 21:46:05 - 1.7 +++ merge.c 6 May 2014 20:03:52 - @@ -40,7 +40,6 @@ merge_main(int argc, char **argv) BUF *bp; flags = labels = 0; - status = D_ERROR; /* * Using getopt(3) and not rcs_getopt() because merge(1) Index: rcs.c === RCS file: /cvs/src/usr.bin/rcs/rcs.c,v retrieving revision 1.80 diff -u -p -r1.80 rcs.c --- rcs.c 7 Jan 2014 14:08:16 - 1.80 +++ rcs.c 6 May 2014 20:06:00 - @@ -214,7 +214,6 @@ rcs_write(RCSFILE *rfp) int fd; fn = NULL; - fd = -1; if (rfp-rf_flags RCS_SYNCED) return; Index: rcsmerge.c === RCS file: /cvs/src/usr.bin/rcs/rcsmerge.c,v retrieving revision 1.52 diff -u -p -r1.52 rcsmerge.c --- rcsmerge.c 23 Jul 2010 21:46:05 - 1.52 +++ rcsmerge.c 6 May 2014 20:08:02 - @@ -44,7 +44,6 @@ rcsmerge_main(int argc, char **argv) BUF *bp; flags = 0; - kflag = RCS_KWEXP_ERR; status = D_ERROR; rev1 = rev2 = NULL; rev_str1 = rev_str2 = NULL; Index: rcsparse.c === RCS file: /cvs/src/usr.bin/rcs/rcsparse.c,v retrieving revision 1.9 diff -u -p -r1.9 rcsparse.c --- rcsparse.c 3 Jun 2013 17:04:35 - 1.9 +++ rcsparse.c 6 May 2014 20:10:23 - @@ -915,7 +915,6 @@ rcsparse_token(RCSFILE *rfp, int allowed } while (isspace(c)); pdp-rp_msglineno = pdp-rp_lineno; - type = 0; switch (c) { case '@': ret = rcsparse_string(rfp, allowed); @@ -1104,7 +1103,6 @@ rcsparse(RCSFILE *rfp, struct rcs_sectio int i, token; pdp = (struct rcs_pdata *)rfp-rf_pdata; - i = 0; token = 0; for (i = 0; sec[i].token != 0; i++) { Index: rcsutil.c === RCS file: /cvs/src/usr.bin/rcs/rcsutil.c,v retrieving revision 1.39 diff -u -p -r1.39 rcsutil.c --- rcsutil.c 16 Apr 2013 20:24:45 - 1.39 +++ rcsutil.c 6 May 2014 20:12:13 - @@ -157,8 +157,6 @@ rcs_choosefile(const char *filename, cha char *p, *ext, name[MAXPATHLEN], *next, *ptr, rcsdir[MAXPATHLEN], *suffixes, rcspath[MAXPATHLEN]; - fd = -1; - /* * If `filename' contains a directory, `rcspath' contains that * directory, including a trailing slash. Otherwise `rcspath' Index: rlog.c === RCS file: /cvs/src/usr.bin/rcs/rlog.c,v retrieving revision 1.67 diff -u -p -r1.67 rlog.c --- rlog.c 7 Jan 2014 14:08:16 - 1.67 +++ rlog.c 6 May 2014 20:14:50 - @@ -433,7 +433,7 @@ rlog_rev_print(struct rcs_delta *rdp) struct rcs_branch *rb; struct rcs_delta *nrdp; - i = found = 0; + found = 0; author = NULL; /* -l[lockers] */
[PATCH] mail assignment never read
Hi tech, this assignment is never read. Fritjof Index: collect.c === RCS file: /cvs/src/usr.bin/mail/collect.c,v retrieving revision 1.34 diff -u -p -r1.34 collect.c --- collect.c 17 Jan 2014 18:42:30 - 1.34 +++ collect.c 24 Apr 2014 21:31:38 - @@ -65,7 +65,6 @@ collect(struct header *hp, int printhead collf = NULL; eofcount = 0; hadintr = 0; - lastlong = 0; longline = 0; if ((cp = value(escape)) != NULL) escape = *cp;
[patch] cvs some values never read
Hi tech, there are some set operations, which are never read. Fritjof Index: rcsparse.c === RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v retrieving revision 1.7 diff -u -p -r1.7 rcsparse.c --- rcsparse.c 3 Jun 2013 17:04:35 - 1.7 +++ rcsparse.c 23 Apr 2014 17:09:44 - @@ -916,7 +916,6 @@ rcsparse_token(RCSFILE *rfp, int allowed } while (isspace(c)); pdp-rp_msglineno = pdp-rp_lineno; - type = 0; switch (c) { case '@': ret = rcsparse_string(rfp, allowed); @@ -1105,7 +1104,6 @@ rcsparse(RCSFILE *rfp, struct rcs_sectio int i, token; pdp = (struct rcs_pdata *)rfp-rf_pdata; - i = 0; token = 0; for (i = 0; sec[i].token != 0; i++) { Index: diff_internals.c === RCS file: /cvs/src/usr.bin/cvs/diff_internals.c,v retrieving revision 1.34 diff -u -p -r1.34 diff_internals.c --- diff_internals.c1 Apr 2011 17:25:26 - 1.34 +++ diff_internals.c23 Apr 2014 17:10:19 - @@ -1375,7 +1375,7 @@ dump_unified_vec(FILE *f1, FILE *f2, int if (context_vec_start context_vec_ptr) return; - b = d = 0; /* gcc */ + d = 0; /* gcc */ lowa = MAX(1, cvp-a - diff_context); upb = MIN(len[0], context_vec_ptr-b + diff_context); lowc = MAX(1, cvp-c - diff_context); Index: getlog.c === RCS file: /cvs/src/usr.bin/cvs/getlog.c,v retrieving revision 1.97 diff -u -p -r1.97 getlog.c --- getlog.c8 Jan 2014 13:23:55 - 1.97 +++ getlog.c23 Apr 2014 17:11:59 - @@ -318,7 +318,7 @@ log_rev_print(struct rcs_delta *rdp) struct rcs_branch *rb; struct rcs_delta *nrdp; - i = found = 0; + found = 0; /* -s states */ if (runflags L_STATES) { Index: rcs.c === RCS file: /cvs/src/usr.bin/cvs/rcs.c,v retrieving revision 1.311 diff -u -p -r1.311 rcs.c --- rcs.c 8 Jan 2014 13:23:55 - 1.311 +++ rcs.c 23 Apr 2014 17:21:39 - @@ -293,8 +293,6 @@ rcs_write(RCSFILE *rfp) size_t len; int fd, saved_errno; - fd = -1; - if (rfp-rf_flags RCS_SYNCED) return; @@ -2229,7 +2227,6 @@ rcs_kwexp_line(char *rcsfile, struct rcs return; c = line-l_line; - found = 0; /* Final character in buffer. */ fin = c + len - 1; @@ -2503,7 +2500,6 @@ rcs_kwexp_line(char *rcsfile, struct rcs lp-l_len = strlen(lp-l_line); TAILQ_INSERT_AFTER((lines-l_lines), cur, lp, l_list); - cur = lp; end = line-l_line + line-l_len - 1;
Re: [patch] cvs some values never read
* Fritjof Bornebusch frit...@alokat.org [2014-04-23 19:30]: there are some set operations, which are never read. RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v guess we need to decide what to do with opencvs really. either there is someone who cares and picks it up, or we can straight delete it. it hasn't moved forward in years, and I have a hard time seeing it going anywhere (except Attic). But that's just me, of course. If opencvs is going to be deleted, what is the alternative? gnucvs? -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/ Fritjof
Re: [patch] cvs some values never read
* Fritjof Bornebusch frit...@alokat.org [2014-04-23 20:15]: * Fritjof Bornebusch frit...@alokat.org [2014-04-23 19:30]: there are some set operations, which are never read. RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v guess we need to decide what to do with opencvs really. either there is someone who cares and picks it up, or we can straight delete it. it hasn't moved forward in years, and I have a hard time seeing it going anywhere (except Attic). But that's just me, of course. If opencvs is going to be deleted, what is the alternative? gnucvs? err, that's what we've been using all the time. It has never become ready. revision 1.114 date: 2010/06/26 03:59:34; author: deraadt; state: Exp; lines: +2 -2; disable opencvs; maintainers went bye bye Ah, I see. -- Henning Brauer, h...@bsws.de, henn...@openbsd.org BS Web Services GmbH, http://bsws.de, Full-Service ISP Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed Henning Brauer Consulting, http://henningbrauer.com/
[PATCH] ssh: variables never read
Hi tech, there are some unread set operations in the ssh code. Fritjof Index: clientloop.c === RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v retrieving revision 1.258 diff -u -p -r1.258 clientloop.c --- clientloop.c2 Feb 2014 03:44:31 - 1.258 +++ clientloop.c23 Apr 2014 19:50:18 - @@ -934,7 +934,6 @@ process_cmdline(void) /* XXX update list of forwards in options */ if (delete) { - cancel_port = 0; cancel_host = hpdelim(s); /* may be NULL */ if (s != NULL) { cancel_port = a2port(s); Index: krl.c === RCS file: /cvs/src/usr.bin/ssh/krl.c,v retrieving revision 1.14 diff -u -p -r1.14 krl.c --- krl.c 31 Jan 2014 16:39:19 - 1.14 +++ krl.c 23 Apr 2014 19:55:50 - @@ -496,7 +496,6 @@ choose_next_state(int current_state, u_i if (cost_bitmap_restart cost) { new_state = KRL_SECTION_CERT_SERIAL_BITMAP; *force_new_section = 1; - cost = cost_bitmap_restart; } debug3(%s: contig %llu last_gap %llu next_gap %llu final %d, costs: list %llu range %llu bitmap %llu new bitmap %llu, @@ -957,7 +956,6 @@ ssh_krl_from_blob(Buffer *buf, struct ss /* Not interested for now. */ continue; } - sig_seen = 1; /* First string component is the signing key */ if ((key = key_from_blob(blob, blen)) == NULL) { error(%s: invalid signature key, __func__); Index: readconf.c === RCS file: /cvs/src/usr.bin/ssh/readconf.c,v retrieving revision 1.219 diff -u -p -r1.219 readconf.c --- readconf.c 23 Apr 2014 12:42:34 - 1.219 +++ readconf.c 23 Apr 2014 20:01:26 - @@ -1255,7 +1255,7 @@ parse_int: if (!arg || *arg == '\0') fatal(%.200s line %d: Missing ControlPersist argument., filename, linenum); - value = 0; + value2 = 0; /* timeout */ if (strcmp(arg, no) == 0 || strcmp(arg, false) == 0) value = 0; Index: scp.c === RCS file: /cvs/src/usr.bin/ssh/scp.c,v retrieving revision 1.179 diff -u -p -r1.179 scp.c --- scp.c 20 Nov 2013 20:53:10 - 1.179 +++ scp.c 23 Apr 2014 20:05:22 - @@ -818,7 +818,6 @@ next: if (fd != -1) { if (fd != -1) { if (close(fd) 0 !haderr) haderr = errno; - fd = -1; } if (!haderr) (void) atomicio(vwrite, remout, , 1); Index: sftp.c === RCS file: /cvs/src/usr.bin/ssh/sftp.c,v retrieving revision 1.160 diff -u -p -r1.160 sftp.c --- sftp.c 22 Apr 2014 10:07:12 - 1.160 +++ sftp.c 23 Apr 2014 20:11:54 - @@ -1222,7 +1222,6 @@ parse_args(const char **cpp, int *ignore *aflag = *fflag = *hflag = *iflag = *lflag = *pflag = 0; *rflag = *sflag = 0; *path1 = *path2 = NULL; - optidx = 1; switch (cmdnum) { case I_GET: case I_REGET:
typo security.8
Hi tech, it's Trojan horse not Trojan horsed, right? Fritjof Index: security.8 === RCS file: /cvs/src/share/man/man8/security.8,v retrieving revision 1.23 diff -u -p -r1.23 security.8 --- security.8 20 Apr 2014 22:15:49 - 1.23 +++ security.8 22 Apr 2014 16:25:09 - @@ -84,7 +84,7 @@ to protect the programs in .Pp .Sy Note: These checks do not provide complete protection against -Trojan horsed binaries, as +Trojan horse binaries, as the miscreant can modify the tree specification to match the replaced binary. For details on really protecting yourself against modified binaries, see .Xr mtree 8 .