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]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]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] 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
>
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 -
Date: Sat, 26 Sep 2015 22:00:58 +0200
From: Fritjof Bornebusch
To: Michael Reed
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, &errstr);
> > + 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&view=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&content-type=text/plain&only_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, &errstr);
> 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, &errstr);
> > + 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, &errstr);
> > + 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] 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, &errstr);
> > + 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&view=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&content-type=text/plain&only_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, &errstr);
> 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, &errstr);
> > + 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, &errstr);
> > + if (errstr)
> > + fatal("invalid job number");
> > + requ[requests++] = i;
> > } else {
> > if (users >= MAXUSERS)
> > fatal("Too many users");
> >
>
[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, &errstr);
+ 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, &errstr);
+ 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, &errstr);
+ 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
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]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] 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
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));
+
[patch] return instead of exit(3) in src/bin/
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.
--F.
Index: chio.c
===
RCS file: /cvs/src/bin/chio/chio.c,v
retrieving revision 1.25
diff -u -p -r1.25 chio.c
--- chio.c 16 Mar 2014 18:38:30 - 1.25
+++ chio.c 30 Aug 2015 16:52:27 -
@@ -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.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.34
diff -u -p -r1.34 chmod.c
--- chmod.c 25 Jun 2015 02:04:08 - 1.34
+++ chmod.c 30 Aug 2015 16:58:54 -
@@ -279,7 +279,7 @@ done:
if (errno)
err(1, "fts_read");
fts_close(ftsp);
- exit(rval);
+ return (rval);
}
/*
Index: date.c
===
RCS file: /cvs/src/bin/date/date.c,v
retrieving revision 1.47
diff -u -p -r1.47 date.c
--- date.c 17 Apr 2015 16:47:47 - 1.47
+++ date.c 30 Aug 2015 17:04:57 -
@@ -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.c
===
RCS file: /cvs/src/bin/dd/dd.c,v
retrieving revision 1.21
diff -u -p -r1.21 dd.c
--- dd.c16 Jan 2015 06:39:31 - 1.21
+++ dd.c30 Aug 2015 17:07:35 -
@@ -85,7 +85,7 @@ main(int argc, char *argv[])
}
dd_close();
- exit(0);
+ return (0);
}
static void
Index: df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.52
diff -u -p -r1.52 df.c
--- df.c16 Jan 2015 06:39:31 - 1.52
+++ df.c30 Aug 2015 17:09:25 -
@@ -175,7 +175,7 @@ main(int argc, char *argv[])
bsdprint(mntbuf, mntsize, maxwidth);
}
- exit(mntsize ? 0 : 1);
+ return (mntsize ? 0 : 1);
}
char *
Index: domainname.c
===
RCS file: /cvs/src/bin/domainname/domainname.c,v
retrieving revision 1.9
diff -u -p -r1.9 domainname.c
--- domainname.c16 Jan 2015 06:39:31 - 1.9
+++ domainname.c30 Aug 2015 17:11:29 -
@@ -66,7 +66,7 @@ main(int argc, char *argv[])
err(1, "getdomainname");
(void)printf("%s\n", domainname);
}
- exit(0);
+ return (0);
}
void
Index: expr.c
===
RCS file: /cvs/src/bin/expr/expr.c,v
retrieving revision 1.20
diff -u -p -r1.20 expr.c
--- expr.c 11 Aug 2015 17:15:46 - 1.20
+++ expr.c 30 Aug 2015 17:18:27 -
@@ -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(vp));
}
Index: hostname.c
===
RCS file: /cvs/src/bin/hostname/hostname.c,v
retrieving revision 1.9
diff -u -p -r1.9 hostname.c
--- hostname.c 16 Jan 2015 06:39:32 - 1.9
+++ hostname.c 30 Aug 2015 17:19:29 -
@@ -72,7 +72,7 @@ main(int argc, char *argv[])
*p = '\0';
(void)printf("%s\n", hostname);
}
- exit(0);
+ return (0);
}
void
Index: kill.c
===
RCS file: /cvs/src/bin/kill/kill.c,v
retrieving revision 1.12
diff -u -p -r1.12 kill.c
--- kill.c 23 Mar 2014 12:44:00 - 1.12
+++ kill.c 30 Aug 2015 17:22:55 -
@@ -74,10 +74,10 @@ main(int argc, char *argv[])
if (numsig <= 0 || numsig >= NSIG)
nosig(*argv);
printf("%s\n", sys_signame[numsig]);
- exit(0);
+ return (0);
}
printsignals(stdout);
- exit(0);
+ return (0);
}
if (!strcmp(*argv, "-s")) {
@@ -126,7 +126,7 @@ main(int argc, char *argv[])
}
}
- exit(errors);
+ ret
[patch] cat's main never return
Hi, just saw that cat's *main* function does never return even though there is a return value, exit(3) is called instead. Is there any reason why or is it just historically, cause it's a bit confusing? If exit(3) is always called, than why not changing the return value to *void*? Other calls in *src/bin/* share the same behavior. Regards, --F. Index: cat.c === RCS file: /cvs/src/bin/cat/cat.c,v retrieving revision 1.21 diff -u -p -r1.21 cat.c --- cat.c 16 Jan 2015 06:39:28 - 1.21 +++ cat.c 29 Aug 2015 22:01:35 - @@ -103,8 +103,7 @@ main(int argc, char *argv[]) raw_args(argv); if (fclose(stdout)) err(1, "stdout"); - exit(rval); - /* NOTREACHED */ + return (rval); } void
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;
> &g
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 > > >
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: 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: 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" 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: 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
[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
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]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;
[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
#include
#include
#include
@@ -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;
}
[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]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;
}
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
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
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: 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]
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;
}
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
#include
#include
+#include
#include
#include
@@ -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 '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&q
[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
#include
#include
+#include
#include
#include
@@ -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 '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
#include
#include
+#include
#include
#include
@@ -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 '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 -
@@ -554,7
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]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
> 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
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
[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] 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
#include
+#include
#include
#include
#include
@@ -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
#include
+#include
#include
#include
#include
@@ -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] 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
-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]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
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: 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
[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: 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 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 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: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]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: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 > 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 > > > > > > > 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 -l
[patch] rcs: stored values never read
Hi tech,
according to scan-build(1) there are a few "never read" values.
fritjof
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.118
diff -u -p -r1.118 co.c
--- co.c2 Oct 2014 06:23:15 - 1.118
+++ co.c6 Oct 2014 18:55:54 -
@@ -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;
@@ -256,7 +255,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 Oct 2014 19:03:31 -
@@ -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.8
diff -u -p -r1.8 merge.c
--- merge.c 2 Oct 2014 06:23:15 - 1.8
+++ merge.c 6 Oct 2014 19:04:07 -
@@ -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 Oct 2014 19:05:56 -
@@ -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.53
diff -u -p -r1.53 rcsmerge.c
--- rcsmerge.c 2 Oct 2014 06:23:15 - 1.53
+++ rcsmerge.c 6 Oct 2014 19:07:52 -
@@ -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 Oct 2014 19:10:03 -
@@ -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.40
diff -u -p -r1.40 rcsutil.c
--- rcsutil.c 29 May 2014 16:39:42 - 1.40
+++ rcsutil.c 6 Oct 2014 19:11:42 -
@@ -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.68
diff -u -p -r1.68 rlog.c
--- rlog.c 2 Oct 2014 06:23:15 - 1.68
+++ rlog.c 6 Oct 2014 19:14:00 -
@@ -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]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);
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)) {
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)();
- ex
[Patch] use exit() directly in usage()
Hi,
after usage() was called, there is no where you can go.
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.c2 Jun 2014 22:18:25 -
@@ -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.c2 Jun 2014 22:19:38 -
@@ -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 2 Jun 2014 22:20:55 -
@@ -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 2 Jun 2014 22:22: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)();
- 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
[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]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
> +#include
> #include
>
> #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->
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]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);
&g
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] 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);
>
[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);
[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
+#include
#include
#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;
-
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 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 = dat
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
> 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 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
> /\
>
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
[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; }
[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]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);
}
Re: [PATCH] rcs: no way to go, after usage was called
Any comments?
On Thu, May 08, 2014 at 10:17:15PM +0200, Fritjof Bornebusch wrote:
> 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 @@ rcs
Re: [PATCH] rcs regression tests
Any feedback? On Thu, May 15, 2014 at 12:07:56AM +0200, Fritjof Bornebusch wrote: > 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 > = >
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] remove dirty if from rlog.c
Hi tech,
there is a dirty if statement in rlog.c, that checks if there is a valid
locker, state or writer and returns if not.
With help from jca - thanks for that - I removed the dirty if statement and
check for valid data in the sections.
I tested it and it behaves like the previous one with the dirty if - I hope I
didn't missed something.
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 25 May 2014 22:24:39 -
@@ -426,20 +426,16 @@ rlog_file(const char *fname, RCSFILE *fi
static void
rlog_rev_print(struct rcs_delta *rdp)
{
- int i, found;
+ int i, found_locker, found_state, found_writer;
struct tm t;
- char *author, numb[RCS_REV_BUFSZ], *fmt, timeb[RCS_TIME_BUFSZ];
+ char numb[RCS_REV_BUFSZ], *fmt, timeb[RCS_TIME_BUFSZ];
struct rcs_argvector *largv, *sargv, *wargv;
struct rcs_branch *rb;
struct rcs_delta *nrdp;
- i = found = 0;
- author = NULL;
-
/* -l[lockers] */
if (lflag == 1) {
- if (rdp->rd_locker != NULL)
- found++;
+ found_locker = 0;
if (llist != NULL) {
/* if locker is empty, no need to go further. */
@@ -449,57 +445,59 @@ rlog_rev_print(struct rcs_delta *rdp)
for (i = 0; largv->argv[i] != NULL; i++) {
if (strcmp(rdp->rd_locker, largv->argv[i])
== 0) {
- found++;
+ found_locker = 1;
break;
}
- found = 0;
}
rcs_argv_destroy(largv);
- }
+ } else if (rdp->rd_locker != NULL)
+ found_locker = 1;
+ if (!found_locker)
+ return;
}
/* -sstates */
if (slist != NULL) {
+ found_state = 0;
+
sargv = rcs_strsplit(slist, ",");
for (i = 0; sargv->argv[i] != NULL; i++) {
if (strcmp(rdp->rd_state, sargv->argv[i]) == 0) {
- found++;
+ found_state = 1;
break;
}
- found = 0;
}
rcs_argv_destroy(sargv);
+
+ if (!found_state)
+ return;
}
/* -w[logins] */
if (wflag == 1) {
+ found_writer = 0;
+
if (wlist != NULL) {
wargv = rcs_strsplit(wlist, ",");
for (i = 0; wargv->argv[i] != NULL; i++) {
- if (strcmp(rdp->rd_author, wargv->argv[i])
- == 0) {
- found++;
+ if (!strcmp(rdp->rd_author, wargv->argv[i])) {
+ found_writer = 1;
break;
}
- found = 0;
}
rcs_argv_destroy(wargv);
} else {
+ char*author;
+
if ((author = getlogin()) == NULL)
err(1, "getlogin");
if (strcmp(rdp->rd_author, author) == 0)
- found++;
+ found_writer = 1;
}
+ if (!found_writer)
+ return;
}
-
- /* XXX dirty... */
- if slist != NULL && wflag == 1) ||
- (slist != NULL && lflag == 1) ||
- (lflag == 1 && wflag == 1)) && found < 2) ||
- (((slist != NULL && lflag == 1 && wflag == 1) ||
- (slist != NULL || lflag == 1 || wflag == 1)) && found == 0))
- return;
printf("%s\n", REVSEP);
[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 Fri, May 09, 2014 at 06:01:52PM +0200, J??r??mie Courr??ges-Anglas wrote:
> Fritjof Bornebusch 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] 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 wrote: > > On 9 May 2014 11:41, Fritjof Bornebusch 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 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 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 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
[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 A
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 : > > > 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 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;
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 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] 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 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
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
