Re: [patch] security(8) and spamd blacklist

2017-06-29 Thread Fritjof Bornebusch
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

2017-06-29 Thread Fritjof Bornebusch
Hi,

security(8) iterates over /var/mail and check is the files belong to the
owner of the same name. So far so good, but spamd.conf.5 says:

override:\
 :white:\
 :method=file:\
 :file=/var/mail/override.txt:

myblack:\
:black:\
:msg=/var/mail/myblackmsg.txt:\
:method=file:\
:file=/var/mail/myblack.txt:

so the user *black.txt* and/or *override.txt* are assumed to exist
by security(8). As it says:

Checking mailbox ownership.
user myblack.txt mailbox is owned by _spamd 

The following diff documents this in the manpage of spamd.conf(5) by
changing the path to /var/mail/_spamd/.

I thought about changing security(8) to fix this, but _spamd is the name
of the user, so it does whats it's supposed to do.

Comments? Because the notification above is very annoying.

--f.

Index: spamd.conf.5
===
RCS file: /cvs/src/share/man/man5/spamd.conf.5,v
retrieving revision 1.19
diff -u -p -r1.19 spamd.conf.5
--- spamd.conf.516 Mar 2017 15:09:32 -  1.19
+++ spamd.conf.529 Jun 2017 19:30:27 -
@@ -65,13 +65,13 @@ nixspam:\e
 override:\e
:white:\e
:method=file:\e
-   :file=/var/mail/override.txt:
+   :file=/var/mail/_spamd/override.txt:
 
 myblack:\e
:black:\e
-   :msg=/var/mail/myblackmsg.txt:\e
+   :msg=/var/mail/_spamd/myblackmsg.txt:\e
:method=file:\e
-   :file=/var/mail/myblack.txt:
+   :file=/var/mail/_spamd/myblack.txt:
 .Ed
 .Pp
 The default configuration file must include the entry



Re: [patch] return instead of exit(3) in src/bin/

2015-11-08 Thread Fritjof Bornebusch
On Mon, Aug 31, 2015 at 10:59:36PM +0200, Fritjof Bornebusch wrote:

Ping 

> On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote:
> > As suggested by deraadt@ and tobias@ it might be better to use the *return* 
> > statement instead of exit(3) 
> > inside the *main* function, to let the stack protector do its work.
> > 
> > This diff removes such calls in all *src/bin/* tools, except those who 
> > already use it.
> > I think I didn't miss a call and didn't introduce any bugs.
> >
> 
> New diff with help from tobias@, as theo@ pointed me to a downside.
> 
> > --F.
> > 
> 
> 
> 
> Index: cat/cat.c
> ===
> RCS file: /cvs/src/bin/cat/cat.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 cat.c
> --- cat/cat.c 16 Jan 2015 06:39:28 -  1.21
> +++ cat/cat.c 31 Aug 2015 20:44:20 -
> @@ -103,8 +103,7 @@ main(int argc, char *argv[])
>   raw_args(argv);
>   if (fclose(stdout))
>   err(1, "stdout");
> - exit(rval);
> - /* NOTREACHED */
> + return (rval);
>  }
>  
>  void
> Index: chio/chio.c
> ===
> RCS file: /cvs/src/bin/chio/chio.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 chio.c
> --- chio/chio.c   16 Mar 2014 18:38:30 -  1.25
> +++ chio/chio.c   31 Aug 2015 20:44:21 -
> @@ -148,7 +148,7 @@ main(int argc, char *argv[])
>   if (commands[i].cc_name == NULL)
>   errx(1, "unknown command: %s", *argv);
>  
> - exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
> + return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
>  }
>  
>  static int
> Index: chmod/chmod.c
> ===
> RCS file: /cvs/src/bin/chmod/chmod.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 chmod.c
> --- chmod/chmod.c 25 Jun 2015 02:04:08 -  1.34
> +++ chmod/chmod.c 31 Aug 2015 20:44:22 -
> @@ -279,7 +279,7 @@ done:
>   if (errno)
>   err(1, "fts_read");
>   fts_close(ftsp);
> - exit(rval);
> + return (rval);
>  }
>  
>  /*
> Index: cp/cp.c
> ===
> RCS file: /cvs/src/bin/cp/cp.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 cp.c
> --- cp/cp.c   7 May 2015 17:32:20 -   1.38
> +++ cp/cp.c   31 Aug 2015 20:44:22 -
> @@ -224,7 +224,7 @@ main(int argc, char *argv[])
>   type = FILE_TO_DIR;
>   }
>  
> - exit(copy(argv, type, fts_options));
> + return (copy(argv, type, fts_options));
>  }
>  
>  char *
> Index: date/date.c
> ===
> RCS file: /cvs/src/bin/date/date.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 date.c
> --- date/date.c   17 Apr 2015 16:47:47 -  1.47
> +++ date/date.c   31 Aug 2015 20:44:22 -
> @@ -143,7 +143,7 @@ main(int argc, char *argv[])
>   errx(1, "conversion error");
>   (void)strftime(buf, sizeof(buf), format, tp);
>   (void)printf("%s\n", buf);
> - exit(0);
> + return (0);
>  }
>  
>  #define  ATOI2(ar)   ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - 
> '0'))
> Index: dd/dd.c
> ===
> RCS file: /cvs/src/bin/dd/dd.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 dd.c
> --- dd/dd.c   16 Jan 2015 06:39:31 -  1.21
> +++ dd/dd.c   31 Aug 2015 20:44:22 -
> @@ -85,7 +85,7 @@ main(int argc, char *argv[])
>   }
>  
>   dd_close();
> - exit(0);
> + return (0);
>  }
>  
>  static void
> Index: df/df.c
> ===
> RCS file: /cvs/src/bin/df/df.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 df.c
> --- df/df.c   16 Jan 2015 06:39:31 -  1.52
> +++ df/df.c   31 Aug 2015 20:44:23 -
> @@ -175,7 +175,7 @@ main(int argc, char *argv[])
>   bsdprint(mntbuf, mntsize, maxwidth);
>   }
>  
> - exit(mntsize ? 0 : 1);
> + return (mntsize ? 0 : 1);
>  }
>  
>  char *
> Index: domainname/domainname.c
> ===
> RCS file: /cvs/src/bin/domainname/domainname.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 domainname.c
> --- domainname/domainname.c   16 Jan 2015 06:39:31 -  1.9
> +++ domainname/domainname.c   

Re: [patch]rcs: usage functions above the main ones

2015-11-08 Thread Fritjof Bornebusch
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]

2015-11-08 Thread Fritjof Bornebusch
- Forwarded message from Fritjof Bornebusch <frit...@alokat.org> -

Date: Sat, 26 Sep 2015 22:00:58 +0200
From: Fritjof Bornebusch <frit...@alokat.org>
To: Michael Reed <m.r...@mykolab.com>
Cc: tech@openbsd.org
Subject: Re: [patch] lpr atoi -> strtonum

On Fri, Sep 25, 2015 at 02:23:21PM -0400, Michael Reed wrote:

Ping 

> Hi Fritjof,
> 

Hi Michael,

> I left one comment inline.
> 

thanks.

> On 09/25/15 08:18, Fritjof Bornebusch wrote:
> > Hi,
> > 
> > change atoi(3) -> strtonum(3) in lpr(1) and lprm(1).
> > lprm(1) avoids negative numbers to be the first argument by using getopt(3),
> > but supported values like 2.2.
> > 
> > --F.
> > 
> > 
> > Index: lpr/lpr.c
> > ===
> > RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v
> > retrieving revision 1.48
> > diff -u -p -r1.48 lpr.c
> > --- lpr/lpr.c   9 Feb 2015 23:00:14 -   1.48
> > +++ lpr/lpr.c   25 Sep 2015 12:08:57 -
> > @@ -112,6 +112,7 @@ main(int argc, char **argv)
> > char buf[PATH_MAX];
> > int i, f, ch;
> > struct stat stb;
> > +   const char *errstr;
> >  
> > /*
> >  * Simulate setuid daemon w/ PRIV_END called.
> > @@ -145,11 +146,11 @@ main(int argc, char **argv)
> > switch (ch) {
> >  
> > case '#':   /* n copies */
> > -   if (isdigit((unsigned char)*optarg)) {
> > -   i = atoi(optarg);
> > -   if (i > 0)
> > -   ncopies = i;
> > -   }
> > +   i = strtonum(optarg, 0, INT_MAX, );
> > +   if (errstr)
> > +   errx(1, "invalid quantity number");
> > +   if (i > 0)
> > +   ncopies = i;
> 
> I might be missing something, but why silently allow -#0 ?

The default value is 1 and if -#0 is called, this default value is used.
Disallow -#0 silently makes this default value useless. And other
BSD versions of lpr(1) uses this default value:

https://svnweb.freebsd.org/base/head/usr.sbin/lpr/lpr/lpr.c?revision=275855=co
http://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/usr.sbin/lpr/lpr/lpr.c
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/usr.sbin/lpr/lpr/lpr.c?rev=1.46=text/plain_with_tag=MAIN

So I think this behavior should not be changed. The above versions redirect 
negative values to 1 as well,
but calling -#-1 looks wrong (what should lpr(1) print, if -1 copies are 
requested), so I think starting from
0 is okay. 

> Besides that, this isn't as informative as it could be IMO; perhaps
> this is better:
> 
> case '#': /* n copies */
>   ncopies = strtonum(optarg, 1, INT_MAX, );
>   if (errstr)
>   errx(1, "number of copies %s: %s", errstr, optarg);
>   break;
> 
> > break;
> >  
> > case '4':   /* troff fonts */
> > @@ -203,7 +204,9 @@ main(int argc, char **argv)
> >  
> > case 'i':   /* indent output */
> > iflag++;
> > -   indent = atoi(optarg);
> > +   indent = strtonum(optarg, 0, INT_MAX, );
> > +   if (errstr)
> > +   errx(1, "invalid number");
> > if (indent < 0)
> > indent = 8;
> > break;
> > Index: lprm/lprm.c
> > ===
> > RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 lprm.c
> > --- lprm/lprm.c 16 Jan 2015 06:40:18 -  1.21
> > +++ lprm/lprm.c 25 Sep 2015 12:08:57 -
> > @@ -77,8 +77,9 @@ main(int argc, char **argv)
> >  {
> > struct passwd *pw;
> > char *cp;
> > +   const char *errstr;
> > long l;
> > -   int ch;
> > +   int ch, i;
> >  
> > /*
> >  * Simulate setuid daemon w/ PRIV_END called.
> > @@ -135,7 +136,10 @@ main(int argc, char **argv)
> > if (isdigit((unsigned char)*argv[0])) {
> > if (requests >= MAXREQUESTS)
> > fatal("Too many requests");
> > -   requ[requests++] = atoi(argv[0]);
> > +   i = strtonum(argv[0], 0, INT_MAX, );
> > +   if (errstr)
> > +   fatal("invalid job number");
> > +   requ[requests++] = i;
> > } else {
> > if (users >= MAXUSERS)
> > fatal("Too many users");
> > 
> 


- End forwarded message -



Re: [patch]rcs: mark unlink as (void)

2015-11-08 Thread Fritjof Bornebusch
On Mon, Jun 15, 2015 at 09:56:18PM +0200, Fritjof Bornebusch wrote:

Ping ...

> Hi tech@,
> 
> mark this unlink(2) call as *(void)*, as there is no need to check the return 
> value.
> This makes it more consistent to all other unlink(2) calls, since they are 
> marked as *(void)* as
> well.
> 
> Regards,
> --F.
> 
> 
> Index: co.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/co.c,v
> retrieving revision 1.121
> diff -u -p -r1.121 co.c
> --- co.c  13 Jun 2015 20:15:21 -  1.121
> +++ co.c  15 Jun 2015 19:50:12 -
> @@ -553,7 +553,7 @@ checkout_file_has_diffs(RCSFILE *rfp, RC
>   ret = diffreg(dst, tempfile, bp, D_FORCEASCII);
>  
>   buf_free(bp);
> - unlink(tempfile);
> + (void)unlink(tempfile);
>   free(tempfile);
>  
>   return (ret);
> 



Re: [patch]apmd ? sign

2015-11-08 Thread Fritjof Bornebusch
On Wed, May 20, 2015 at 05:08:21PM +0200, Fritjof Bornebusch wrote:

Ping 

> 
> Index: apmd.c
> ===
> RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 apmd.c
> --- apmd.c6 Feb 2015 08:16:50 -   1.75
> +++ apmd.c20 May 2015 15:04:38 -
> @@ -403,7 +403,6 @@ main(int argc, char *argv[])
>   doperf = PERF_MANUAL;
>   setperfpolicy("high");
>   break;
> - case '?':
>   default:
>   usage();
>   }




Re: [patch]diff: uninitialized values

2015-11-08 Thread Fritjof Bornebusch
On Wed, Jun 17, 2015 at 09:19:28PM +0200, Fritjof Bornebusch wrote:
> On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote:
> > Hi tech@,
> > 
> > *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called.
> >
> 
> Such initializers hiding a false positive, cause the compiler does not 
> understand this case can never happen.
> -> warning: 'edp1' may be used uninitialized in this function
> -> warning: 'edp2' may be used uninitialized in this function
> 
> Sorry for beeing not that clear.
>

Ping 
  
> > Regards,
> > --F.
> > 
> > 
> > Index: diffdir.c
> > ===
> > RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 diffdir.c
> > --- diffdir.c   16 Jan 2015 06:40:07 -  1.43
> > +++ diffdir.c   17 Jun 2015 18:50:57 -
> > @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char
> >  void
> >  diffdir(char *p1, char *p2, int flags)
> >  {
> > -   struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL;
> > -   struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL;
> > +   struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL;
> > +   struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL;
> > size_t dirlen1, dirlen2;
> > char path1[PATH_MAX], path2[PATH_MAX];
> > int pos;
> > 
> 



Re: [patch] lpr atoi -> strtonum

2015-09-26 Thread Fritjof Bornebusch
On Fri, Sep 25, 2015 at 02:23:21PM -0400, Michael Reed wrote:
> Hi Fritjof,
> 

Hi Michael,

> I left one comment inline.
> 

thanks.

> On 09/25/15 08:18, Fritjof Bornebusch wrote:
> > Hi,
> > 
> > change atoi(3) -> strtonum(3) in lpr(1) and lprm(1).
> > lprm(1) avoids negative numbers to be the first argument by using getopt(3),
> > but supported values like 2.2.
> > 
> > --F.
> > 
> > 
> > Index: lpr/lpr.c
> > ===
> > RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v
> > retrieving revision 1.48
> > diff -u -p -r1.48 lpr.c
> > --- lpr/lpr.c   9 Feb 2015 23:00:14 -   1.48
> > +++ lpr/lpr.c   25 Sep 2015 12:08:57 -
> > @@ -112,6 +112,7 @@ main(int argc, char **argv)
> > char buf[PATH_MAX];
> > int i, f, ch;
> > struct stat stb;
> > +   const char *errstr;
> >  
> > /*
> >  * Simulate setuid daemon w/ PRIV_END called.
> > @@ -145,11 +146,11 @@ main(int argc, char **argv)
> > switch (ch) {
> >  
> > case '#':   /* n copies */
> > -   if (isdigit((unsigned char)*optarg)) {
> > -   i = atoi(optarg);
> > -   if (i > 0)
> > -   ncopies = i;
> > -   }
> > +   i = strtonum(optarg, 0, INT_MAX, );
> > +   if (errstr)
> > +   errx(1, "invalid quantity number");
> > +   if (i > 0)
> > +   ncopies = i;
> 
> I might be missing something, but why silently allow -#0 ?

The default value is 1 and if -#0 is called, this default value is used.
Disallow -#0 silently makes this default value useless. And other
BSD versions of lpr(1) uses this default value:

https://svnweb.freebsd.org/base/head/usr.sbin/lpr/lpr/lpr.c?revision=275855=co
http://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/usr.sbin/lpr/lpr/lpr.c
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/usr.sbin/lpr/lpr/lpr.c?rev=1.46=text/plain_with_tag=MAIN

So I think this behavior should not be changed. The above versions redirect 
negative values to 1 as well,
but calling -#-1 looks wrong (what should lpr(1) print, if -1 copies are 
requested), so I think starting from
0 is okay. 

> Besides that, this isn't as informative as it could be IMO; perhaps
> this is better:
> 
> case '#': /* n copies */
>   ncopies = strtonum(optarg, 1, INT_MAX, );
>   if (errstr)
>   errx(1, "number of copies %s: %s", errstr, optarg);
>   break;
> 
> > break;
> >  
> > case '4':   /* troff fonts */
> > @@ -203,7 +204,9 @@ main(int argc, char **argv)
> >  
> > case 'i':   /* indent output */
> > iflag++;
> > -   indent = atoi(optarg);
> > +   indent = strtonum(optarg, 0, INT_MAX, );
> > +   if (errstr)
> > +   errx(1, "invalid number");
> > if (indent < 0)
> > indent = 8;
> > break;
> > Index: lprm/lprm.c
> > ===
> > RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 lprm.c
> > --- lprm/lprm.c 16 Jan 2015 06:40:18 -  1.21
> > +++ lprm/lprm.c 25 Sep 2015 12:08:57 -
> > @@ -77,8 +77,9 @@ main(int argc, char **argv)
> >  {
> > struct passwd *pw;
> > char *cp;
> > +   const char *errstr;
> > long l;
> > -   int ch;
> > +   int ch, i;
> >  
> > /*
> >  * Simulate setuid daemon w/ PRIV_END called.
> > @@ -135,7 +136,10 @@ main(int argc, char **argv)
> > if (isdigit((unsigned char)*argv[0])) {
> > if (requests >= MAXREQUESTS)
> > fatal("Too many requests");
> > -   requ[requests++] = atoi(argv[0]);
> > +   i = strtonum(argv[0], 0, INT_MAX, );
> > +   if (errstr)
> > +   fatal("invalid job number");
> > +   requ[requests++] = i;
> > } else {
> > if (users >= MAXUSERS)
> > fatal("Too many users");
> > 
> 



[patch] lpr style

2015-09-25 Thread Fritjof Bornebusch
Hi,

this diff changes the following:

- exit(3) to return at the end of main functions
- use /* NOTREACHED */ were it belongs according to style(9)
- lpc.c and lpd.c lack a return at the end of the main functions, as the main 
loops exists the
  program. I'm not sure if this is a "coders choise" argument, so correct me if 
I'm wrong. 

--F.

Index: filters/lpf.c
===
RCS file: /cvs/src/usr.sbin/lpr/filters/lpf.c,v
retrieving revision 1.13
diff -u -p -r1.13 lpf.c
--- filters/lpf.c   9 Feb 2015 23:00:14 -   1.13
+++ filters/lpf.c   25 Sep 2015 11:14:02 -
@@ -98,6 +98,7 @@ main(int argc, char **argv) 
break;
default:
usage();
+   /* NOTREACHED */
}
}
argc -= optind;
@@ -206,7 +207,8 @@ main(int argc, char **argv) 
freopen(acctfile, "a", stdout) != NULL) {
printf("%7.2f\t%s:%s\n", (float)npages, host, name);
}
-   exit(0);
+
+   return (0);
 }
 
 __dead void
Index: lpc/lpc.c
===
RCS file: /cvs/src/usr.sbin/lpr/lpc/lpc.c,v
retrieving revision 1.19
diff -u -p -r1.19 lpc.c
--- lpc/lpc.c   16 Jan 2015 06:40:18 -  1.19
+++ lpc/lpc.c   25 Sep 2015 11:14:02 -
@@ -103,6 +103,8 @@ main(int argc, char **argv)
signal(SIGINT, intr);
for (;;)
cmdscanner();
+
+   return 0;
 }
 
 volatile sig_atomic_t gotintr;
Index: lpd/lpd.c
===
RCS file: /cvs/src/usr.sbin/lpr/lpd/lpd.c,v
retrieving revision 1.58
diff -u -p -r1.58 lpd.c
--- lpd/lpd.c   9 Feb 2015 23:00:14 -   1.58
+++ lpd/lpd.c   25 Sep 2015 11:14:02 -
@@ -203,7 +203,7 @@ main(int argc, char **argv)
break;
default:
usage();
-   break;
+   /* NOTREACHED */
}
}
argc -= optind;
@@ -223,6 +223,7 @@ main(int argc, char **argv)
break;
default:
usage();
+   /* NOTREACHED */
}
 
 #ifndef DEBUG
@@ -419,6 +420,8 @@ main(int argc, char **argv)
}
(void)close(s);
}
+
+   return 0;
 }
 
 static void
Index: lpq/lpq.c
===
RCS file: /cvs/src/usr.sbin/lpr/lpq/lpq.c,v
retrieving revision 1.22
diff -u -p -r1.22 lpq.c
--- lpq/lpq.c   9 Feb 2015 23:00:14 -   1.22
+++ lpq/lpq.c   25 Sep 2015 11:14:02 -
@@ -108,6 +108,7 @@ main(int argc, char **argv)
case '?':
default:
usage();
+   /* NOTREACHED */
}
}
 
@@ -145,7 +146,8 @@ main(int argc, char **argv)
}
} else
displayq(lflag);
-   exit(0);
+
+   return (0);
 }
 
 /* XXX - could be common w/ lpd */
Index: lpr/lpr.c
===
RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v
retrieving revision 1.48
diff -u -p -r1.48 lpr.c
--- lpr/lpr.c   9 Feb 2015 23:00:14 -   1.48
+++ lpr/lpr.c   25 Sep 2015 11:14:02 -
@@ -238,6 +238,7 @@ main(int argc, char **argv)
 
default:
usage();
+   /* NOTREACHED */
}
}
argc -= optind;
@@ -389,7 +390,6 @@ main(int argc, char **argv)
}
cleanup(0);
return (1);
-   /* NOTREACHED */
 }
 
 /*
Index: lprm/lprm.c
===
RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v
retrieving revision 1.21
diff -u -p -r1.21 lprm.c
--- lprm/lprm.c 16 Jan 2015 06:40:18 -  1.21
+++ lprm/lprm.c 25 Sep 2015 11:14:02 -
@@ -122,6 +122,7 @@ main(int argc, char **argv)
break;
default:
usage();
+   /* NOTREACHED */
}
}
argc -= optind;
@@ -146,7 +147,7 @@ main(int argc, char **argv)
}
 
rmjob();
-   exit(0);
+   return (0);
 }
 
 static __dead void
Index: lptest/lptest.c
===
RCS file: /cvs/src/usr.sbin/lpr/lptest/lptest.c,v
retrieving revision 1.8
diff -u -p -r1.8 lptest.c
--- lptest/lptest.c 27 Oct 2009 23:59:52 -  1.8
+++ lptest/lptest.c 25 Sep 2015 11:14:02 -
@@ -66,5 +66,5 @@ main(int argc, char **argv)
putchar('\n');
}
(void)fflush(stdout);
-   exit(0);
+   return (0);
 }
Index: pac/pac.c
===
RCS file: /cvs/src/usr.sbin/lpr/pac/pac.c,v
retrieving revision 1.25
diff -u -p -r1.25 pac.c
--- pac/pac.c  

[patch] lpr atoi -> strtonum

2015-09-25 Thread Fritjof Bornebusch
Hi,

change atoi(3) -> strtonum(3) in lpr(1) and lprm(1).
lprm(1) avoids negative numbers to be the first argument by using getopt(3),
but supported values like 2.2.

--F.


Index: lpr/lpr.c
===
RCS file: /cvs/src/usr.sbin/lpr/lpr/lpr.c,v
retrieving revision 1.48
diff -u -p -r1.48 lpr.c
--- lpr/lpr.c   9 Feb 2015 23:00:14 -   1.48
+++ lpr/lpr.c   25 Sep 2015 12:08:57 -
@@ -112,6 +112,7 @@ main(int argc, char **argv)
char buf[PATH_MAX];
int i, f, ch;
struct stat stb;
+   const char *errstr;
 
/*
 * Simulate setuid daemon w/ PRIV_END called.
@@ -145,11 +146,11 @@ main(int argc, char **argv)
switch (ch) {
 
case '#':   /* n copies */
-   if (isdigit((unsigned char)*optarg)) {
-   i = atoi(optarg);
-   if (i > 0)
-   ncopies = i;
-   }
+   i = strtonum(optarg, 0, INT_MAX, );
+   if (errstr)
+   errx(1, "invalid quantity number");
+   if (i > 0)
+   ncopies = i;
break;
 
case '4':   /* troff fonts */
@@ -203,7 +204,9 @@ main(int argc, char **argv)
 
case 'i':   /* indent output */
iflag++;
-   indent = atoi(optarg);
+   indent = strtonum(optarg, 0, INT_MAX, );
+   if (errstr)
+   errx(1, "invalid number");
if (indent < 0)
indent = 8;
break;
Index: lprm/lprm.c
===
RCS file: /cvs/src/usr.sbin/lpr/lprm/lprm.c,v
retrieving revision 1.21
diff -u -p -r1.21 lprm.c
--- lprm/lprm.c 16 Jan 2015 06:40:18 -  1.21
+++ lprm/lprm.c 25 Sep 2015 12:08:57 -
@@ -77,8 +77,9 @@ main(int argc, char **argv)
 {
struct passwd *pw;
char *cp;
+   const char *errstr;
long l;
-   int ch;
+   int ch, i;
 
/*
 * Simulate setuid daemon w/ PRIV_END called.
@@ -135,7 +136,10 @@ main(int argc, char **argv)
if (isdigit((unsigned char)*argv[0])) {
if (requests >= MAXREQUESTS)
fatal("Too many requests");
-   requ[requests++] = atoi(argv[0]);
+   i = strtonum(argv[0], 0, INT_MAX, );
+   if (errstr)
+   fatal("invalid job number");
+   requ[requests++] = i;
} else {
if (users >= MAXUSERS)
fatal("Too many users");



Re: [patch]diff: uninitialized values

2015-09-18 Thread Fritjof Bornebusch
On Wed, Jun 17, 2015 at 09:19:28PM +0200, Fritjof Bornebusch wrote:
> On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote:
> > Hi tech@,
> > 
> > *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called.
> >
> 
> Such initializers hiding a false positive, cause the compiler does not 
> understand this case can never happen.
> -> warning: 'edp1' may be used uninitialized in this function
> -> warning: 'edp2' may be used uninitialized in this function
> 
> Sorry for beeing not that clear.
>

Ping 
  
> > Regards,
> > --F.
> > 
> > 
> > Index: diffdir.c
> > ===
> > RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 diffdir.c
> > --- diffdir.c   16 Jan 2015 06:40:07 -  1.43
> > +++ diffdir.c   17 Jun 2015 18:50:57 -
> > @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char
> >  void
> >  diffdir(char *p1, char *p2, int flags)
> >  {
> > -   struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL;
> > -   struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL;
> > +   struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL;
> > +   struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL;
> > size_t dirlen1, dirlen2;
> > char path1[PATH_MAX], path2[PATH_MAX];
> > int pos;
> > 
> 



Re: [patch]rcs: mark unlink as (void)

2015-09-18 Thread Fritjof Bornebusch
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

2015-09-18 Thread Fritjof Bornebusch
On Mon, Jun 15, 2015 at 11:42:10AM +0100, Nicholas Marriott wrote:
> 
> this seems fine to me
>

Ping ...
 
> 
> On Sun, Jun 14, 2015 at 10:38:40PM +0200, Fritjof Bornebusch wrote:
> > Hi tech@,
> > 
> > most of the tools implements the *usage* function above the *main* function.
> > This patch makes it more consistent to these tools and where the different 
> > *usage*
> > functions are implemented in rcs in general.
> > 
> > Any comments?
> > 
> > Regards,
> > --F.
> > 
> > 
> > Index: co.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/co.c,v
> > retrieving revision 1.121
> > diff -u -p -r1.121 co.c
> > --- co.c13 Jun 2015 20:15:21 -  1.121
> > +++ co.c14 Jun 2015 20:21:41 -
> > @@ -43,6 +43,17 @@ static void  checkout_err_nobranch(RCSFIL
> >  const char *, int);
> >  static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *);
> >  
> > +__dead void
> > +checkout_usage(void)
> > +{
> > +   fprintf(stderr,
> > +   "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n"
> > +   "  [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n"
> > +   "  [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n");
> > +   
> > +   exit(1);
> > +}
> > +
> >  int
> >  checkout_main(int argc, char **argv)
> >  {
> > @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv)
> > }
> >  
> > return (ret);
> > -}
> > -
> > -__dead void
> > -checkout_usage(void)
> > -{
> > -   fprintf(stderr,
> > -   "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n"
> > -   "  [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n"
> > -   "  [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n");
> > -   
> > -   exit(1);
> >  }
> >  
> >  /*
> > Index: ident.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/ident.c,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 ident.c
> > --- ident.c 2 Oct 2014 06:23:15 -   1.30
> > +++ ident.c 14 Jun 2015 20:21:41 -
> > @@ -41,6 +41,14 @@ static int flags = 0;
> >  static voidident_file(const char *, FILE *);
> >  static voidident_line(FILE *);
> >  
> > +__dead void
> > +ident_usage(void)
> > +{
> > +   fprintf(stderr, "usage: ident [-qV] [file ...]\n");
> > +   
> > +   exit(1);
> > +}
> > +
> >  int
> >  ident_main(int argc, char **argv)
> >  {
> > @@ -158,12 +166,4 @@ ident_line(FILE *fp)
> >  out:
> > if (bp != NULL)
> > buf_free(bp);
> > -}
> > -
> > -__dead void
> > -ident_usage(void)
> > -{
> > -   fprintf(stderr, "usage: ident [-qV] [file ...]\n");
> > -   
> > -   exit(1);
> >  }
> > Index: merge.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/merge.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 merge.c
> > --- merge.c 10 Oct 2014 08:15:25 -  1.9
> > +++ merge.c 14 Jun 2015 20:21:41 -
> > @@ -32,6 +32,15 @@
> >  #include "rcsprog.h"
> >  #include "diff.h"
> >  
> > +__dead void
> > +merge_usage(void)
> > +{
> > +   fprintf(stderr,
> > +   "usage: merge [-EepqV] [-L label] file1 file2 file3\n");
> > +
> > +   exit(D_ERROR);
> > +}
> > +
> >  int
> >  merge_main(int argc, char **argv)
> >  {
> > @@ -108,13 +117,4 @@ merge_main(int argc, char **argv)
> > buf_free(bp);
> >  
> > return (status);
> > -}
> > -
> > -__dead void
> > -merge_usage(void)
> > -{
> > -   (void)fprintf(stderr,
> > -   "usage: merge [-EepqV] [-L label] file1 file2 file3\n");
> > -
> > -   exit(D_ERROR);
> >  }
> > Index: rcsclean.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
> > retrieving revision 1.54
> > diff -u -p -r1.54 rcsclean.c
> > --- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
> > +++ rcsclean.c  14 Jun 2015 20:21:41 -
> > @@ -43,6 +43,16 @@ static int uflag = 0;
> >  static int flags = 0;
> &g

Re: [patch] return instead of exit(3) in src/bin/

2015-09-17 Thread Fritjof Bornebusch
On Mon, Aug 31, 2015 at 10:59:36PM +0200, Fritjof Bornebusch wrote:
> On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote:
> > As suggested by deraadt@ and tobias@ it might be better to use the *return* 
> > statement instead of exit(3) 
> > inside the *main* function, to let the stack protector do its work.
> > 
> > This diff removes such calls in all *src/bin/* tools, except those who 
> > already use it.
> > I think I didn't miss a call and didn't introduce any bugs.
> >
> 
> New diff with help from tobias@, as theo@ pointed me to a downside.
> 
> > --F.
> > 
> 

Any comments or feedback?

> 
> 
> Index: cat/cat.c
> ===
> RCS file: /cvs/src/bin/cat/cat.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 cat.c
> --- cat/cat.c 16 Jan 2015 06:39:28 -  1.21
> +++ cat/cat.c 31 Aug 2015 20:44:20 -
> @@ -103,8 +103,7 @@ main(int argc, char *argv[])
>   raw_args(argv);
>   if (fclose(stdout))
>   err(1, "stdout");
> - exit(rval);
> - /* NOTREACHED */
> + return (rval);
>  }
>  
>  void
> Index: chio/chio.c
> ===
> RCS file: /cvs/src/bin/chio/chio.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 chio.c
> --- chio/chio.c   16 Mar 2014 18:38:30 -  1.25
> +++ chio/chio.c   31 Aug 2015 20:44:21 -
> @@ -148,7 +148,7 @@ main(int argc, char *argv[])
>   if (commands[i].cc_name == NULL)
>   errx(1, "unknown command: %s", *argv);
>  
> - exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
> + return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
>  }
>  
>  static int
> Index: chmod/chmod.c
> ===
> RCS file: /cvs/src/bin/chmod/chmod.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 chmod.c
> --- chmod/chmod.c 25 Jun 2015 02:04:08 -  1.34
> +++ chmod/chmod.c 31 Aug 2015 20:44:22 -
> @@ -279,7 +279,7 @@ done:
>   if (errno)
>   err(1, "fts_read");
>   fts_close(ftsp);
> - exit(rval);
> + return (rval);
>  }
>  
>  /*
> Index: cp/cp.c
> ===
> RCS file: /cvs/src/bin/cp/cp.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 cp.c
> --- cp/cp.c   7 May 2015 17:32:20 -   1.38
> +++ cp/cp.c   31 Aug 2015 20:44:22 -
> @@ -224,7 +224,7 @@ main(int argc, char *argv[])
>   type = FILE_TO_DIR;
>   }
>  
> - exit(copy(argv, type, fts_options));
> + return (copy(argv, type, fts_options));
>  }
>  
>  char *
> Index: date/date.c
> ===
> RCS file: /cvs/src/bin/date/date.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 date.c
> --- date/date.c   17 Apr 2015 16:47:47 -  1.47
> +++ date/date.c   31 Aug 2015 20:44:22 -
> @@ -143,7 +143,7 @@ main(int argc, char *argv[])
>   errx(1, "conversion error");
>   (void)strftime(buf, sizeof(buf), format, tp);
>   (void)printf("%s\n", buf);
> - exit(0);
> + return (0);
>  }
>  
>  #define  ATOI2(ar)   ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - 
> '0'))
> Index: dd/dd.c
> ===
> RCS file: /cvs/src/bin/dd/dd.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 dd.c
> --- dd/dd.c   16 Jan 2015 06:39:31 -  1.21
> +++ dd/dd.c   31 Aug 2015 20:44:22 -
> @@ -85,7 +85,7 @@ main(int argc, char *argv[])
>   }
>  
>   dd_close();
> - exit(0);
> + return (0);
>  }
>  
>  static void
> Index: df/df.c
> ===
> RCS file: /cvs/src/bin/df/df.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 df.c
> --- df/df.c   16 Jan 2015 06:39:31 -  1.52
> +++ df/df.c   31 Aug 2015 20:44:23 -
> @@ -175,7 +175,7 @@ main(int argc, char *argv[])
>   bsdprint(mntbuf, mntsize, maxwidth);
>   }
>  
> - exit(mntsize ? 0 : 1);
> + return (mntsize ? 0 : 1);
>  }
>  
>  char *
> Index: domainname/domainname.c
> ===
> RCS file: /cvs/src/bin/domainname/domainname.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 domainname.c
> --- domainname/domainname.c   16 Jan 2015 06:39:31 -  1.9
> +++ domai

Re: [patch] return instead of exit(3) in src/bin/

2015-08-31 Thread Fritjof Bornebusch
On Sun, Aug 30, 2015 at 08:01:02PM +0200, Fritjof Bornebusch wrote:
> As suggested by deraadt@ and tobias@ it might be better to use the *return* 
> statement instead of exit(3) 
> inside the *main* function, to let the stack protector do its work.
> 
> This diff removes such calls in all *src/bin/* tools, except those who 
> already use it.
> I think I didn't miss a call and didn't introduce any bugs.
>

New diff with help from tobias@, as theo@ pointed me to a downside.

> --F.
> 



Index: cat/cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.21
diff -u -p -r1.21 cat.c
--- cat/cat.c   16 Jan 2015 06:39:28 -  1.21
+++ cat/cat.c   31 Aug 2015 20:44:20 -
@@ -103,8 +103,7 @@ main(int argc, char *argv[])
raw_args(argv);
if (fclose(stdout))
err(1, "stdout");
-   exit(rval);
-   /* NOTREACHED */
+   return (rval);
 }
 
 void
Index: chio/chio.c
===
RCS file: /cvs/src/bin/chio/chio.c,v
retrieving revision 1.25
diff -u -p -r1.25 chio.c
--- chio/chio.c 16 Mar 2014 18:38:30 -  1.25
+++ chio/chio.c 31 Aug 2015 20:44:21 -
@@ -148,7 +148,7 @@ main(int argc, char *argv[])
if (commands[i].cc_name == NULL)
errx(1, "unknown command: %s", *argv);
 
-   exit((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
+   return ((*commands[i].cc_handler)(commands[i].cc_name, argc, argv));
 }
 
 static int
Index: chmod/chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.34
diff -u -p -r1.34 chmod.c
--- chmod/chmod.c   25 Jun 2015 02:04:08 -  1.34
+++ chmod/chmod.c   31 Aug 2015 20:44:22 -
@@ -279,7 +279,7 @@ done:
if (errno)
err(1, "fts_read");
fts_close(ftsp);
-   exit(rval);
+   return (rval);
 }
 
 /*
Index: cp/cp.c
===
RCS file: /cvs/src/bin/cp/cp.c,v
retrieving revision 1.38
diff -u -p -r1.38 cp.c
--- cp/cp.c 7 May 2015 17:32:20 -   1.38
+++ cp/cp.c 31 Aug 2015 20:44:22 -
@@ -224,7 +224,7 @@ main(int argc, char *argv[])
type = FILE_TO_DIR;
}
 
-   exit(copy(argv, type, fts_options));
+   return (copy(argv, type, fts_options));
 }
 
 char *
Index: date/date.c
===
RCS file: /cvs/src/bin/date/date.c,v
retrieving revision 1.47
diff -u -p -r1.47 date.c
--- date/date.c 17 Apr 2015 16:47:47 -  1.47
+++ date/date.c 31 Aug 2015 20:44:22 -
@@ -143,7 +143,7 @@ main(int argc, char *argv[])
errx(1, "conversion error");
(void)strftime(buf, sizeof(buf), format, tp);
(void)printf("%s\n", buf);
-   exit(0);
+   return (0);
 }
 
 #defineATOI2(ar)   ((ar) += 2, ((ar)[-2] - '0') * 10 + ((ar)[-1] - 
'0'))
Index: dd/dd.c
===
RCS file: /cvs/src/bin/dd/dd.c,v
retrieving revision 1.21
diff -u -p -r1.21 dd.c
--- dd/dd.c 16 Jan 2015 06:39:31 -  1.21
+++ dd/dd.c 31 Aug 2015 20:44:22 -
@@ -85,7 +85,7 @@ main(int argc, char *argv[])
}
 
dd_close();
-   exit(0);
+   return (0);
 }
 
 static void
Index: df/df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.52
diff -u -p -r1.52 df.c
--- df/df.c 16 Jan 2015 06:39:31 -  1.52
+++ df/df.c 31 Aug 2015 20:44:23 -
@@ -175,7 +175,7 @@ main(int argc, char *argv[])
bsdprint(mntbuf, mntsize, maxwidth);
}
 
-   exit(mntsize ? 0 : 1);
+   return (mntsize ? 0 : 1);
 }
 
 char *
Index: domainname/domainname.c
===
RCS file: /cvs/src/bin/domainname/domainname.c,v
retrieving revision 1.9
diff -u -p -r1.9 domainname.c
--- domainname/domainname.c 16 Jan 2015 06:39:31 -  1.9
+++ domainname/domainname.c 31 Aug 2015 20:44:23 -
@@ -66,7 +66,7 @@ main(int argc, char *argv[])
err(1, "getdomainname");
(void)printf("%s\n", domainname);
}
-   exit(0);
+   return (0);
 }
 
 void
Index: expr/expr.c
===
RCS file: /cvs/src/bin/expr/expr.c,v
retrieving revision 1.20
diff -u -p -r1.20 expr.c
--- expr/expr.c 11 Aug 2015 17:15:46 -  1.20
+++ expr/expr.c 31 Aug 2015 20:44:23 -
@@ -518,5 +518,5 @@ main(int argc, char *argv[])
else
printf("%s\n", vp->u.s);
 
-   exit(is_zero_or_null(vp));
+   return (is_zero_or_null

Re: [patch]rcs: mark unlink as (void)

2015-06-23 Thread Fritjof Bornebusch
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

2015-06-23 Thread Fritjof Bornebusch
On Mon, Jun 15, 2015 at 11:42:10AM +0100, Nicholas Marriott wrote:
 
 this seems fine to me


Ping 
 
 
 On Sun, Jun 14, 2015 at 10:38:40PM +0200, Fritjof Bornebusch wrote:
  Hi tech@,
  
  most of the tools implements the *usage* function above the *main* function.
  This patch makes it more consistent to these tools and where the different 
  *usage*
  functions are implemented in rcs in general.
  
  Any comments?
  
  Regards,
  --F.
  
  
  Index: co.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/co.c,v
  retrieving revision 1.121
  diff -u -p -r1.121 co.c
  --- co.c13 Jun 2015 20:15:21 -  1.121
  +++ co.c14 Jun 2015 20:21:41 -
  @@ -43,6 +43,17 @@ static void  checkout_err_nobranch(RCSFIL
   const char *, int);
   static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *);
   
  +__dead void
  +checkout_usage(void)
  +{
  +   fprintf(stderr,
  +   usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
  + [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
  + [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
  +   
  +   exit(1);
  +}
  +
   int
   checkout_main(int argc, char **argv)
   {
  @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv)
  }
   
  return (ret);
  -}
  -
  -__dead void
  -checkout_usage(void)
  -{
  -   fprintf(stderr,
  -   usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
  - [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
  - [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
  -   
  -   exit(1);
   }
   
   /*
  Index: ident.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/ident.c,v
  retrieving revision 1.30
  diff -u -p -r1.30 ident.c
  --- ident.c 2 Oct 2014 06:23:15 -   1.30
  +++ ident.c 14 Jun 2015 20:21:41 -
  @@ -41,6 +41,14 @@ static int flags = 0;
   static voidident_file(const char *, FILE *);
   static voidident_line(FILE *);
   
  +__dead void
  +ident_usage(void)
  +{
  +   fprintf(stderr, usage: ident [-qV] [file ...]\n);
  +   
  +   exit(1);
  +}
  +
   int
   ident_main(int argc, char **argv)
   {
  @@ -158,12 +166,4 @@ ident_line(FILE *fp)
   out:
  if (bp != NULL)
  buf_free(bp);
  -}
  -
  -__dead void
  -ident_usage(void)
  -{
  -   fprintf(stderr, usage: ident [-qV] [file ...]\n);
  -   
  -   exit(1);
   }
  Index: merge.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/merge.c,v
  retrieving revision 1.9
  diff -u -p -r1.9 merge.c
  --- merge.c 10 Oct 2014 08:15:25 -  1.9
  +++ merge.c 14 Jun 2015 20:21:41 -
  @@ -32,6 +32,15 @@
   #include rcsprog.h
   #include diff.h
   
  +__dead void
  +merge_usage(void)
  +{
  +   fprintf(stderr,
  +   usage: merge [-EepqV] [-L label] file1 file2 file3\n);
  +
  +   exit(D_ERROR);
  +}
  +
   int
   merge_main(int argc, char **argv)
   {
  @@ -108,13 +117,4 @@ merge_main(int argc, char **argv)
  buf_free(bp);
   
  return (status);
  -}
  -
  -__dead void
  -merge_usage(void)
  -{
  -   (void)fprintf(stderr,
  -   usage: merge [-EepqV] [-L label] file1 file2 file3\n);
  -
  -   exit(D_ERROR);
   }
  Index: rcsclean.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
  retrieving revision 1.54
  diff -u -p -r1.54 rcsclean.c
  --- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
  +++ rcsclean.c  14 Jun 2015 20:21:41 -
  @@ -43,6 +43,16 @@ static int uflag = 0;
   static int flags = 0;
   static char *locker = NULL;
   
  +__dead void
  +rcsclean_usage(void)
  +{
  +   fprintf(stderr,
  +   usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n
  +   [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n);
  +
  +   exit(1);
  +}
  +
   int
   rcsclean_main(int argc, char **argv)
   {
  @@ -116,16 +126,6 @@ rcsclean_main(int argc, char **argv)
  rcsclean_file(argv[i], rev_str);
   
  return (0);
  -}
  -
  -__dead void
  -rcsclean_usage(void)
  -{
  -   fprintf(stderr,
  -   usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n
  -   [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n);
  -
  -   exit(1);
   }
   
   static void
  Index: rcsdiff.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
  retrieving revision 1.83
  diff -u -p -r1.83 rcsdiff.c
  --- rcsdiff.c   13 Jun 2015 20:15:21 -  1.83
  +++ rcsdiff.c   14 Jun 2015 20:21:41 -
  @@ -45,6 +45,16 @@ static int quiet;
   static int kflag = RCS_KWEXP_ERR;
   static char *diff_ignore_pats;
   
  +__dead void
  +rcsdiff_usage(void)
  +{
  +   fprintf(stderr,
  +   usage: rcsdiff [-cnquV] [-kmode] [-rrev] [-xsuffixes] [-ztz]\n

Re: [patch]rcs: rlog manpage typo

2015-06-18 Thread Fritjof Bornebusch
On Thu, Jun 18, 2015 at 10:49:32PM +0200, Pablo Méndez Hernández wrote:
Hi,
 
El 18/6/2015 22:46, Fritjof Bornebusch frit...@alokat.org escribiA^3:

 Hi tech@,

 *logins is omitted* sounds a little strange, doesn't it?
 
logins as a keyword?


Yes, but if you read it, it sounds wrong in some way.

Regards.
Pablo


Regards,
--F.
 
 Regards,
 --F.


 Index: rlog.1
 ===
 RCS file: /cvs/src/usr.bin/rcs/rlog.1,v
 retrieving revision 1.24
 diff -u -p -r1.24 rlog.1
 --- rlog.1A  A  A  3 Sep 2010 11:09:29 -A  A  A  A 1.24
 +++ rlog.1A  A  A  18 Jun 2015 20:41:40 -
 @@ -145,7 +145,7 @@ Print information about revisions checke
 A in a comma-separated list.
 A If
 A .Ar logins
 -is omitted, the user's login is assumed.
 +are omitted, the user's login is assumed.
 A .It Fl x Ns Ar suffixes
 A Specifies the suffixes for RCS files.
 A Suffixes should be separated by the




[patch]rcs: merge typo

2015-06-18 Thread Fritjof Bornebusch
What about this comma.
I saw a few manpages, having it at this location.

Regards,
--F.


Index: merge.1
===
RCS file: /cvs/src/usr.bin/rcs/merge.1,v
retrieving revision 1.3
diff -u -p -r1.3 merge.1
--- merge.1 28 Oct 2010 15:08:50 -  1.3
+++ merge.1 18 Jun 2015 20:52:30 -
@@ -45,7 +45,7 @@ for details.
 .It Fl e
 Same as
 .Fl E
-but does not warn about conflicts.
+, but does not warn about conflicts.
 .It Fl L Ar label
 Specifies labels to be used in place of corresponding file names
 in conflict reports.



Re: [patch]rcs: ci manpage typo

2015-06-18 Thread Fritjof Bornebusch
On Thu, Jun 18, 2015 at 09:48:23PM +0100, Jason McIntyre wrote:
 On Thu, Jun 18, 2015 at 10:33:53PM +0200, Fritjof Bornebusch wrote:
  Hi tech@,
  
  isn't there a comma missing?
  
 
 depends how you like your commas. if i were writing it, i'd have the
 comma. but many wouldn;t, and it's certainly not wrong.
 
 the reason i like that last comma is demonstrated with this wonderful
 dedication:
 
   To my parents, Ayn Rand and God.
 
 love it ;)
 jmc
 
 ps won;t be changing the doc though. i don;t see the point of enforcing
 this - it;s really a matter of preference.


I see. Thanks for the explanation.

  Regards,
  --F.
  
  
  Index: ci.1
  ===
  RCS file: /cvs/src/usr.bin/rcs/ci.1,v
  retrieving revision 1.38
  diff -u -p -r1.38 ci.1
  --- ci.112 Aug 2013 14:19:53 -  1.38
  +++ ci.118 Jun 2015 20:30:22 -
  @@ -102,7 +102,7 @@ Only do update check-in.
   Print error and refuse to do check-in if the RCS file does not already 
  exist.
   .It Fl k Ns Op Ar rev
   Search the working file for keywords and set the revision number,
  -creation date, state and author to the values found in these keywords
  +creation date, state, and author to the values found in these keywords
   instead of computing them.
   .It Fl l Ns Op Ar rev
   The same as
  
 



[patch]rcs: ci manpage typo

2015-06-18 Thread Fritjof Bornebusch
Hi tech@,

isn't there a comma missing?

Regards,
--F.


Index: ci.1
===
RCS file: /cvs/src/usr.bin/rcs/ci.1,v
retrieving revision 1.38
diff -u -p -r1.38 ci.1
--- ci.112 Aug 2013 14:19:53 -  1.38
+++ ci.118 Jun 2015 20:30:22 -
@@ -102,7 +102,7 @@ Only do update check-in.
 Print error and refuse to do check-in if the RCS file does not already exist.
 .It Fl k Ns Op Ar rev
 Search the working file for keywords and set the revision number,
-creation date, state and author to the values found in these keywords
+creation date, state, and author to the values found in these keywords
 instead of computing them.
 .It Fl l Ns Op Ar rev
 The same as



[patch]rcs: rlog manpage typo

2015-06-18 Thread Fritjof Bornebusch
Hi tech@,

*logins is omitted* sounds a little strange, doesn't it?

Regards,
--F.


Index: rlog.1
===
RCS file: /cvs/src/usr.bin/rcs/rlog.1,v
retrieving revision 1.24
diff -u -p -r1.24 rlog.1
--- rlog.1  3 Sep 2010 11:09:29 -   1.24
+++ rlog.1  18 Jun 2015 20:41:40 -
@@ -145,7 +145,7 @@ Print information about revisions checke
 in a comma-separated list.
 If
 .Ar logins
-is omitted, the user's login is assumed.
+are omitted, the user's login is assumed.
 .It Fl x Ns Ar suffixes
 Specifies the suffixes for RCS files.
 Suffixes should be separated by the



Re: [patch]rcs: rlog manpage typo

2015-06-18 Thread Fritjof Bornebusch
On Thu, Jun 18, 2015 at 09:57:13PM +0100, Jason McIntyre wrote:
 On Thu, Jun 18, 2015 at 10:44:07PM +0200, Fritjof Bornebusch wrote:
  Hi tech@,
  
  *logins is omitted* sounds a little strange, doesn't it?
  
 
 it does, because in your head you're thinking of logins as being the
 plural of login (i.e. more than one login). but you can also - correctly -
 think of logins as being the argument name.
 
 you could read the text below as:
 
   If [the argument] logins is omitted...
 
 sounds different now, eh?
 
 in the docs, there is often this overlap where we can think of the
 argument name in more than one way. it creates ambiguities, but there
 you go. it would be impractical to try and force one style, i think.
 
 still, for this reason i dislike argument names which take the plural
 form.
 

Yeah, this can cause some confusion. But if it's the style, I'm okay
with it. ;)

 jmc
 

Regards,
--F.

  Regards,
  --F.
  
  
  Index: rlog.1
  ===
  RCS file: /cvs/src/usr.bin/rcs/rlog.1,v
  retrieving revision 1.24
  diff -u -p -r1.24 rlog.1
  --- rlog.1  3 Sep 2010 11:09:29 -   1.24
  +++ rlog.1  18 Jun 2015 20:41:40 -
  @@ -145,7 +145,7 @@ Print information about revisions checke
   in a comma-separated list.
   If
   .Ar logins
  -is omitted, the user's login is assumed.
  +are omitted, the user's login is assumed.
   .It Fl x Ns Ar suffixes
   Specifies the suffixes for RCS files.
   Suffixes should be separated by the
  
 



[patch]diff: xstrdup wrappes strdup(3)

2015-06-17 Thread Fritjof Bornebusch
Hi tech@,

as requested by nicm@, xstrdup calls strdup(3) now.

Regards,
--F.


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/diff/xmalloc.c,v
retrieving revision 1.6
diff -u -p -r1.6 xmalloc.c
--- xmalloc.c   29 Apr 2015 04:00:25 -  1.6
+++ xmalloc.c   17 Jun 2015 18:13:25 -
@@ -73,12 +73,10 @@ xfree(void *ptr)
 char *
 xstrdup(const char *str)
 {
-   size_t len;
char *cp;
-
-   len = strlen(str) + 1;
-   cp = xmalloc(len);
-   strlcpy(cp, str, len);
+   
+   if ((cp = strdup(str)) == NULL)
+   err(1, xstrdup);
return cp;
 }
 



[patch]diff: uninitialized values

2015-06-17 Thread Fritjof Bornebusch
Hi tech@,

*edp1* and *edp2* could be used uninitialized, if *goto closem;* is called.

Regards,
--F.


Index: diffdir.c
===
RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
retrieving revision 1.43
diff -u -p -r1.43 diffdir.c
--- diffdir.c   16 Jan 2015 06:40:07 -  1.43
+++ diffdir.c   17 Jun 2015 18:50:57 -
@@ -48,8 +48,8 @@ static void diffit(struct dirent *, char
 void
 diffdir(char *p1, char *p2, int flags)
 {
-   struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL;
-   struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL;
+   struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL;
+   struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL;
size_t dirlen1, dirlen2;
char path1[PATH_MAX], path2[PATH_MAX];
int pos;



Re: [patch]diff: uninitialized values

2015-06-17 Thread Fritjof Bornebusch
On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote:
 Hi tech@,
 
 *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called.


Such initializers hiding a false positive, cause the compiler does not 
understand this case can never happen.
- warning: 'edp1' may be used uninitialized in this function
- warning: 'edp2' may be used uninitialized in this function

Sorry for beeing not that clear.
 
 Regards,
 --F.
 
 
 Index: diffdir.c
 ===
 RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
 retrieving revision 1.43
 diff -u -p -r1.43 diffdir.c
 --- diffdir.c 16 Jan 2015 06:40:07 -  1.43
 +++ diffdir.c 17 Jun 2015 18:50:57 -
 @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char
  void
  diffdir(char *p1, char *p2, int flags)
  {
 - struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL;
 - struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL;
 + struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL;
 + struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL;
   size_t dirlen1, dirlen2;
   char path1[PATH_MAX], path2[PATH_MAX];
   int pos;
 



[patch]file: xstrdup just wrappes strdup(3)

2015-06-17 Thread Fritjof Bornebusch
Hi tech@,

as requested by nicm@, xstrdup calls strdup(3) now.

Regards,
--F.


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/file/xmalloc.c,v
retrieving revision 1.1
diff -u -p -r1.1 xmalloc.c
--- xmalloc.c   24 Apr 2015 16:24:11 -  1.1
+++ xmalloc.c   17 Jun 2015 18:18:10 -
@@ -76,13 +76,10 @@ xfree(void *ptr)
 char *
 xstrdup(const char *str)
 {
-   size_t len;
char *cp;
 
-   len = strlen(str) + 1;
-   cp = xmalloc(len);
-   if (strlcpy(cp, str, len) = len)
-   errx(1, xstrdup: string truncated);
+   if ((cp = strdup(str)) == NULL)
+   err(1, xstrdup);
return cp;
 }
 



[patch]ssh: xstrdup wrappes strdup(3)

2015-06-17 Thread Fritjof Bornebusch
Hi tech@,

as requested by nicm@, xstrdup just wrappes strdup(3).

Regards,
--F.


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/ssh/xmalloc.c,v
retrieving revision 1.32
diff -u -p -r1.32 xmalloc.c
--- xmalloc.c   24 Apr 2015 01:36:01 -  1.32
+++ xmalloc.c   17 Jun 2015 18:42:43 -
@@ -13,6 +13,7 @@
  * called by a name other than ssh or Secure Shell.
  */
 
+#include errno.h
 #include stdarg.h
 #include stdint.h
 #include stdio.h
@@ -66,12 +67,10 @@ xreallocarray(void *ptr, size_t nmemb, s
 char *
 xstrdup(const char *str)
 {
-   size_t len;
char *cp;
 
-   len = strlen(str) + 1;
-   cp = xmalloc(len);
-   strlcpy(cp, str, len);
+   if ((cp = strdup(str)) == NULL)
+   fatal(xstrdup: %s, strerror(errno));
return cp;
 }
 



Re: [patch]rcs: xstrdup just wrappes strdup

2015-06-17 Thread Fritjof Bornebusch
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)

2015-06-17 Thread Fritjof Bornebusch
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)

2015-06-15 Thread Fritjof Bornebusch
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

2015-06-15 Thread Fritjof Bornebusch
Hi,

thanks for the hint.
This one should do the trick.




Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.9
diff -u -p -r1.9 xmalloc.c
--- xmalloc.c   13 Jun 2015 20:15:21 -  1.9
+++ xmalloc.c   15 Jun 2015 07:52:15 -
@@ -68,13 +68,10 @@ xreallocarray(void *ptr, size_t nmemb, s
 char *
 xstrdup(const char *str)
 {
-   size_t len;
char *cp;
-
-   len = strlen(str) + 1;
-   cp = xmalloc(len);
-   if (strlcpy(cp, str, len) = len)
-   errx(1, xstrdup: string truncated);
+   
+   if ((cp = strdup(str)) == NULL)
+   err(1, xstrdup);
return cp;
 }
 



Re: [patch]rcs: xstrdup just wrappes strdup

2015-06-15 Thread Fritjof Bornebusch
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

2015-06-14 Thread Fritjof Bornebusch
On Wed, Jun 10, 2015 at 07:37:57PM +0200, Fritjof Bornebusch wrote:
 On Wed, May 20, 2015 at 10:55:34AM +0200, Fritjof Bornebusch wrote:
  On Tue, May 19, 2015 at 08:57:06PM +0200, Fritjof Bornebusch wrote:
   Hi,
   
   xstrdup just wrappes strdup, so there is no need to call xmalloc and
   strlcpy instead.
  
 
 
 Ping 


Without PGP stuff 
  
  Use err() instead of errx(), so errno will be printed additionally.
  Thanks to Tim.
   
   Regards,
   --F.
   
   
  
  Regards,
  --F.
  

  


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.8
diff -u -p -r1.8 xmalloc.c
--- xmalloc.c   26 Mar 2015 15:17:30 -  1.8
+++ xmalloc.c   20 May 2015 08:53:01 -
@@ -76,13 +76,11 @@ xfree(void *ptr)
 char *
 xstrdup(const char *str)
 {
-   size_t len;
char *cp;
 
-   len = strlen(str) + 1;
-   cp = xmalloc(len);
-   if (strlcpy(cp, str, len) = len)
-   errx(1, xstrdup: string truncated);
+   if ((cp = strdup(str)) == NULL)
+   err(1, xstrdup: copy string failed);
+
return cp;
 }
 



[patch]rcs: usage functions above the main ones

2015-06-14 Thread Fritjof Bornebusch
Hi tech@,

most of the tools implements the *usage* function above the *main* function.
This patch makes it more consistent to these tools and where the different 
*usage*
functions are implemented in rcs in general.

Any comments?

Regards,
--F.


Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.121
diff -u -p -r1.121 co.c
--- co.c13 Jun 2015 20:15:21 -  1.121
+++ co.c14 Jun 2015 20:21:41 -
@@ -43,6 +43,17 @@ static void  checkout_err_nobranch(RCSFIL
 const char *, int);
 static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *);
 
+__dead void
+checkout_usage(void)
+{
+   fprintf(stderr,
+   usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
+ [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
+ [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+   
+   exit(1);
+}
+
 int
 checkout_main(int argc, char **argv)
 {
@@ -216,17 +227,6 @@ checkout_main(int argc, char **argv)
}
 
return (ret);
-}
-
-__dead void
-checkout_usage(void)
-{
-   fprintf(stderr,
-   usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
- [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
- [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
-   
-   exit(1);
 }
 
 /*
Index: ident.c
===
RCS file: /cvs/src/usr.bin/rcs/ident.c,v
retrieving revision 1.30
diff -u -p -r1.30 ident.c
--- ident.c 2 Oct 2014 06:23:15 -   1.30
+++ ident.c 14 Jun 2015 20:21:41 -
@@ -41,6 +41,14 @@ static int flags = 0;
 static voidident_file(const char *, FILE *);
 static voidident_line(FILE *);
 
+__dead void
+ident_usage(void)
+{
+   fprintf(stderr, usage: ident [-qV] [file ...]\n);
+   
+   exit(1);
+}
+
 int
 ident_main(int argc, char **argv)
 {
@@ -158,12 +166,4 @@ ident_line(FILE *fp)
 out:
if (bp != NULL)
buf_free(bp);
-}
-
-__dead void
-ident_usage(void)
-{
-   fprintf(stderr, usage: ident [-qV] [file ...]\n);
-   
-   exit(1);
 }
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.9
diff -u -p -r1.9 merge.c
--- merge.c 10 Oct 2014 08:15:25 -  1.9
+++ merge.c 14 Jun 2015 20:21:41 -
@@ -32,6 +32,15 @@
 #include rcsprog.h
 #include diff.h
 
+__dead void
+merge_usage(void)
+{
+   fprintf(stderr,
+   usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+   exit(D_ERROR);
+}
+
 int
 merge_main(int argc, char **argv)
 {
@@ -108,13 +117,4 @@ merge_main(int argc, char **argv)
buf_free(bp);
 
return (status);
-}
-
-__dead void
-merge_usage(void)
-{
-   (void)fprintf(stderr,
-   usage: merge [-EepqV] [-L label] file1 file2 file3\n);
-
-   exit(D_ERROR);
 }
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.54
diff -u -p -r1.54 rcsclean.c
--- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
+++ rcsclean.c  14 Jun 2015 20:21:41 -
@@ -43,6 +43,16 @@ static int uflag = 0;
 static int flags = 0;
 static char *locker = NULL;
 
+__dead void
+rcsclean_usage(void)
+{
+   fprintf(stderr,
+   usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n
+   [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n);
+
+   exit(1);
+}
+
 int
 rcsclean_main(int argc, char **argv)
 {
@@ -116,16 +126,6 @@ rcsclean_main(int argc, char **argv)
rcsclean_file(argv[i], rev_str);
 
return (0);
-}
-
-__dead void
-rcsclean_usage(void)
-{
-   fprintf(stderr,
-   usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n
-   [-u[rev]] [-xsuffixes] [-ztz] [file ...]\n);
-
-   exit(1);
 }
 
 static void
Index: rcsdiff.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
retrieving revision 1.83
diff -u -p -r1.83 rcsdiff.c
--- rcsdiff.c   13 Jun 2015 20:15:21 -  1.83
+++ rcsdiff.c   14 Jun 2015 20:21:41 -
@@ -45,6 +45,16 @@ static int quiet;
 static int kflag = RCS_KWEXP_ERR;
 static char *diff_ignore_pats;
 
+__dead void
+rcsdiff_usage(void)
+{
+   fprintf(stderr,
+   usage: rcsdiff [-cnquV] [-kmode] [-rrev] [-xsuffixes] [-ztz]\n
+  [diff_options] file ...\n);
+
+   exit(D_ERROR);
+}
+
 int
 rcsdiff_main(int argc, char **argv)
 {
@@ -262,16 +272,6 @@ rcsdiff_main(int argc, char **argv)
}
 
return (status);
-}
-
-__dead void
-rcsdiff_usage(void)
-{
-   fprintf(stderr,
-   usage: rcsdiff [-cnquV] [-kmode] [-rrev] [-xsuffixes] [-ztz]\n
- 

Re: [patch]rcs: remove xfree

2015-06-13 Thread Fritjof Bornebusch
On Sat, Jun 13, 2015 at 09:33:59AM +0100, Nicholas Marriott wrote:
 Hi. You missed date.y:
 
 date.y: In function 'yyerror':
 date.y:497: error: implicit declaration of function 'xfree'
 

Ups, sorry.
That should do the trick.

 
 On Sat, Jun 13, 2015 at 12:43:29AM +0200, Fritjof Bornebusch wrote:
   Hi tech@,
   
  
  Without PGP / SMIME stuff, sorry.
   
   a couple of months ago I removed the if condition in the *xfree* 
   function, but tedu@ suggested
   that it would be better to remove the *xfree* function entirely instead.
   
   If've seen there are *efree* functions in some tools, that just wrappes 
   the free(3) function call.
   I'm not quite sure what is the best way todo it, so comments are very 
   welcome.

   Regards,
   --F.
  
 
Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.24
diff -u -p -r1.24 buf.c
--- buf.c   5 Feb 2015 12:59:58 -   1.24
+++ buf.c   12 Jun 2015 22:20:32 -
@@ -32,6 +32,7 @@
 #include fcntl.h
 #include stdint.h
 #include stdio.h
+#include stdlib.h
 #include string.h
 #include unistd.h
 
@@ -137,15 +138,14 @@ out:
 void
 buf_free(BUF *b)
 {
-   if (b-cb_buf != NULL)
-   xfree(b-cb_buf);
-   xfree(b);
+   free(b-cb_buf);
+   free(b);
 }
 
 /*
  * Free the buffer b's structural information but do not free the contents
  * of the buffer.  Instead, they are returned and should be freed later using
- * xfree().
+ * free().
  */
 void *
 buf_release(BUF *b)
@@ -153,7 +153,7 @@ buf_release(BUF *b)
void *tmp;
 
tmp = b-cb_buf;
-   xfree(b);
+   free(b);
return (tmp);
 }
 
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.219
diff -u -p -r1.219 ci.c
--- ci.c16 Jan 2015 06:40:11 -  1.219
+++ ci.c12 Jun 2015 22:20:32 -
@@ -211,7 +211,7 @@ checkin_main(int argc, char **argv)
exit(0);
case 'w':
if (pb.author != NULL)
-   xfree(pb.author);
+   free(pb.author);
pb.author = xstrdup(rcs_optarg);
break;
case 'x':
@@ -376,10 +376,8 @@ out:
buf_free(b2);
if (b3 != NULL)
buf_free(b3);
-   if (path1 != NULL)
-   xfree(path1);
-   if (path2 != NULL)
-   xfree(path2);
+   free(path1);
+   free(path2);
 
return (NULL);
 }
@@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb
fprintf(stderr,
reuse log message of previous file? [yn](y): );
if (rcs_yesno('y') != 'y') {
-   xfree(pb-rcs_msg);
+   free(pb-rcs_msg);
pb-rcs_msg = NULL;
}
}
@@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb
pb-username, pb-author, NULL, NULL);
 
if ((pb-flags  INTERACTIVE)  (pb-rcs_msg[0] == '\0')) {
-   xfree(pb-rcs_msg); /* free empty log message */
+   free(pb-rcs_msg);  /* free empty log message */
pb-rcs_msg = NULL;
}
 
@@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[3], tokens[4]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
 
if (i  6)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[5]);
 
if (i  7)
break;
-   if (*state != NULL)
-   xfree(*state);
+   free(*state);
*state = xstrdup(tokens[6]);
break;
case KW_TYPE_AUTHOR:
if (i  2)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[1]);
break;
case KW_TYPE_DATE:
@@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[1], tokens[2]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
break;
case KW_TYPE_STATE:
if (i  2)
break;
-   if (*state != NULL)
-   xfree(*state

[patch]rcs: remove xfree

2015-06-12 Thread Fritjof Bornebusch
 Hi tech@,
 

Without PGP / SMIME stuff, sorry.
 
 a couple of months ago I removed the if condition in the *xfree* function, 
 but tedu@ suggested
 that it would be better to remove the *xfree* function entirely instead.
 
 If've seen there are *efree* functions in some tools, that just wrappes the 
 free(3) function call.
 I'm not quite sure what is the best way todo it, so comments are very welcome.
  
 Regards,
 --F.
 
Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.24
diff -u -p -r1.24 buf.c
--- buf.c   5 Feb 2015 12:59:58 -   1.24
+++ buf.c   12 Jun 2015 22:20:32 -
@@ -32,6 +32,7 @@
 #include fcntl.h
 #include stdint.h
 #include stdio.h
+#include stdlib.h
 #include string.h
 #include unistd.h
 
@@ -137,15 +138,14 @@ out:
 void
 buf_free(BUF *b)
 {
-   if (b-cb_buf != NULL)
-   xfree(b-cb_buf);
-   xfree(b);
+   free(b-cb_buf);
+   free(b);
 }
 
 /*
  * Free the buffer b's structural information but do not free the contents
  * of the buffer.  Instead, they are returned and should be freed later using
- * xfree().
+ * free().
  */
 void *
 buf_release(BUF *b)
@@ -153,7 +153,7 @@ buf_release(BUF *b)
void *tmp;
 
tmp = b-cb_buf;
-   xfree(b);
+   free(b);
return (tmp);
 }
 
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.219
diff -u -p -r1.219 ci.c
--- ci.c16 Jan 2015 06:40:11 -  1.219
+++ ci.c12 Jun 2015 22:20:32 -
@@ -211,7 +211,7 @@ checkin_main(int argc, char **argv)
exit(0);
case 'w':
if (pb.author != NULL)
-   xfree(pb.author);
+   free(pb.author);
pb.author = xstrdup(rcs_optarg);
break;
case 'x':
@@ -376,10 +376,8 @@ out:
buf_free(b2);
if (b3 != NULL)
buf_free(b3);
-   if (path1 != NULL)
-   xfree(path1);
-   if (path2 != NULL)
-   xfree(path2);
+   free(path1);
+   free(path2);
 
return (NULL);
 }
@@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb
fprintf(stderr,
reuse log message of previous file? [yn](y): );
if (rcs_yesno('y') != 'y') {
-   xfree(pb-rcs_msg);
+   free(pb-rcs_msg);
pb-rcs_msg = NULL;
}
}
@@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb
pb-username, pb-author, NULL, NULL);
 
if ((pb-flags  INTERACTIVE)  (pb-rcs_msg[0] == '\0')) {
-   xfree(pb-rcs_msg); /* free empty log message */
+   free(pb-rcs_msg);  /* free empty log message */
pb-rcs_msg = NULL;
}
 
@@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[3], tokens[4]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
 
if (i  6)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[5]);
 
if (i  7)
break;
-   if (*state != NULL)
-   xfree(*state);
+   free(*state);
*state = xstrdup(tokens[6]);
break;
case KW_TYPE_AUTHOR:
if (i  2)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[1]);
break;
case KW_TYPE_DATE:
@@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[1], tokens[2]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
break;
case KW_TYPE_STATE:
if (i  2)
break;
-   if (*state != NULL)
-   xfree(*state);
+   free(*state);
*state = xstrdup(tokens[1]);
break;
case KW_TYPE_REVISION:
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.120
diff -u -p -r1.120 co.c
--- co.c16 Jan 2015 06:40:11 - 

[patch]rcs: remove xfree

2015-06-12 Thread Fritjof Bornebusch
Hi tech@,

a couple of months ago I removed the if condition in the *xfree* function, but 
tedu@ suggested
that it would be better to remove the *xfree* function entirely instead.

If've seen there are *efree* functions in some tools, that just wrappes the 
free(3) function call.
I'm not quite sure what is the best way todo it, so comments are very welcome.

Regards,
--F.


Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.24
diff -u -p -r1.24 buf.c
--- buf.c   5 Feb 2015 12:59:58 -   1.24
+++ buf.c   12 Jun 2015 22:20:32 -
@@ -32,6 +32,7 @@
 #include fcntl.h
 #include stdint.h
 #include stdio.h
+#include stdlib.h
 #include string.h
 #include unistd.h
 
@@ -137,15 +138,14 @@ out:
 void
 buf_free(BUF *b)
 {
-   if (b-cb_buf != NULL)
-   xfree(b-cb_buf);
-   xfree(b);
+   free(b-cb_buf);
+   free(b);
 }
 
 /*
  * Free the buffer b's structural information but do not free the contents
  * of the buffer.  Instead, they are returned and should be freed later using
- * xfree().
+ * free().
  */
 void *
 buf_release(BUF *b)
@@ -153,7 +153,7 @@ buf_release(BUF *b)
void *tmp;
 
tmp = b-cb_buf;
-   xfree(b);
+   free(b);
return (tmp);
 }
 
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.219
diff -u -p -r1.219 ci.c
--- ci.c16 Jan 2015 06:40:11 -  1.219
+++ ci.c12 Jun 2015 22:20:32 -
@@ -211,7 +211,7 @@ checkin_main(int argc, char **argv)
exit(0);
case 'w':
if (pb.author != NULL)
-   xfree(pb.author);
+   free(pb.author);
pb.author = xstrdup(rcs_optarg);
break;
case 'x':
@@ -376,10 +376,8 @@ out:
buf_free(b2);
if (b3 != NULL)
buf_free(b3);
-   if (path1 != NULL)
-   xfree(path1);
-   if (path2 != NULL)
-   xfree(path2);
+   free(path1);
+   free(path2);
 
return (NULL);
 }
@@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb
fprintf(stderr,
reuse log message of previous file? [yn](y): );
if (rcs_yesno('y') != 'y') {
-   xfree(pb-rcs_msg);
+   free(pb-rcs_msg);
pb-rcs_msg = NULL;
}
}
@@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb
pb-username, pb-author, NULL, NULL);
 
if ((pb-flags  INTERACTIVE)  (pb-rcs_msg[0] == '\0')) {
-   xfree(pb-rcs_msg); /* free empty log message */
+   free(pb-rcs_msg);  /* free empty log message */
pb-rcs_msg = NULL;
}
 
@@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[3], tokens[4]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
 
if (i  6)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[5]);
 
if (i  7)
break;
-   if (*state != NULL)
-   xfree(*state);
+   free(*state);
*state = xstrdup(tokens[6]);
break;
case KW_TYPE_AUTHOR:
if (i  2)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[1]);
break;
case KW_TYPE_DATE:
@@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[1], tokens[2]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
break;
case KW_TYPE_STATE:
if (i  2)
break;
-   if (*state != NULL)
-   xfree(*state);
+   free(*state);
*state = xstrdup(tokens[1]);
break;
case KW_TYPE_REVISION:
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.120
diff -u -p -r1.120 co.c
--- co.c16 Jan 2015 06:40:11 -  1.120
+++ co.c12 Jun 2015 22:20:32 

Re: [patch]rcs: xstrdup just wrappes strdup

2015-06-10 Thread Fritjof Bornebusch
On Wed, May 20, 2015 at 10:55:34AM +0200, Fritjof Bornebusch wrote:
 On Tue, May 19, 2015 at 08:57:06PM +0200, Fritjof Bornebusch wrote:
  Hi,
  
  xstrdup just wrappes strdup, so there is no need to call xmalloc and
  strlcpy instead.
 


Ping 
 
 Use err() instead of errx(), so errno will be printed additionally.
 Thanks to Tim.
  
  Regards,
  --F.
  
  
 
 Regards,
 --F.
 
   
 
 
 Index: xmalloc.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
 retrieving revision 1.8
 diff -u -p -r1.8 xmalloc.c
 --- xmalloc.c 26 Mar 2015 15:17:30 -  1.8
 +++ xmalloc.c 20 May 2015 08:53:01 -
 @@ -76,13 +76,11 @@ xfree(void *ptr)
  char *
  xstrdup(const char *str)
  {
 - size_t len;
   char *cp;
  
 - len = strlen(str) + 1;
 - cp = xmalloc(len);
 - if (strlcpy(cp, str, len) = len)
 - errx(1, xstrdup: string truncated);
 + if ((cp = strdup(str)) == NULL)
 + err(1, xstrdup: copy string failed);
 +
   return cp;
  }
  




pgp1q6Ck8BKwd.pgp
Description: PGP signature


Re: [patch]rcs: xstrdup just wrappes strdup

2015-05-20 Thread Fritjof Bornebusch
On Tue, May 19, 2015 at 08:57:06PM +0200, Fritjof Bornebusch wrote:
 Hi,
 
 xstrdup just wrappes strdup, so there is no need to call xmalloc and
 strlcpy instead.


Use err() instead of errx(), so errno will be printed additionally.
Thanks to Tim.
 
 Regards,
 --F.
 
 

Regards,
--F.

  


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.8
diff -u -p -r1.8 xmalloc.c
--- xmalloc.c   26 Mar 2015 15:17:30 -  1.8
+++ xmalloc.c   20 May 2015 08:53:01 -
@@ -76,13 +76,11 @@ xfree(void *ptr)
 char *
 xstrdup(const char *str)
 {
-   size_t len;
char *cp;
 
-   len = strlen(str) + 1;
-   cp = xmalloc(len);
-   if (strlcpy(cp, str, len) = len)
-   errx(1, xstrdup: string truncated);
+   if ((cp = strdup(str)) == NULL)
+   err(1, xstrdup: copy string failed);
+
return cp;
 }
 


pgpW87xQmqoAq.pgp
Description: PGP signature


Re: [patch]apmd ? sign

2015-05-20 Thread Fritjof Bornebusch
On Wed, May 20, 2015 at 09:35:03PM +0200, Alexander Hall wrote:
 
 
 On May 20, 2015 5:08:21 PM GMT+02:00, Fritjof Bornebusch frit...@alokat.org 
 wrote:
 Hi,
 
 for what is the ? sign for?
 
 fallthrough to usage()


But why is this necessary, haven't seen this in other deamons?
BTW: isn't the \* FALLTHROUGH *\ comment missing?
 
 # apmd -?
 
 /Alexander 
 

--F.

 
 Regards,
 --F.
 
 
 Index: apmd.c
 ===
 RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
 retrieving revision 1.75
 diff -u -p -r1.75 apmd.c
 --- apmd.c   6 Feb 2015 08:16:50 -   1.75
 +++ apmd.c   20 May 2015 15:04:38 -
 @@ -403,7 +403,6 @@ main(int argc, char *argv[])
  doperf = PERF_MANUAL;
  setperfpolicy(high);
  break;
 -case '?':
  default:
  usage();
  }
 


pgptou0PoyQly.pgp
Description: PGP signature


[patch]apmd ? sign

2015-05-20 Thread Fritjof Bornebusch
Hi,

for what is the ? sign for?

Regards,
--F.


Index: apmd.c
===
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.75
diff -u -p -r1.75 apmd.c
--- apmd.c  6 Feb 2015 08:16:50 -   1.75
+++ apmd.c  20 May 2015 15:04:38 -
@@ -403,7 +403,6 @@ main(int argc, char *argv[])
doperf = PERF_MANUAL;
setperfpolicy(high);
break;
-   case '?':
default:
usage();
}


pgp1bgAfQehas.pgp
Description: PGP signature


[patch]rcs: xstrdup just wrappes strdup

2015-05-19 Thread Fritjof Bornebusch
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

2015-02-25 Thread Fritjof Bornebusch
On Wed, Dec 24, 2014 at 01:48:44PM +0100, Fritjof Bornebusch wrote:

Ping ..

 Hi tech@,
 
 looks like there are some missing periods regarding the sudo wrong
 password messages.
 
 fritjof
 
 
 Index: ins_csops.h
 ===
 RCS file: /cvs/src/usr.bin/sudo/ins_csops.h,v
 retrieving revision 1.5
 diff -u -p -r1.5 ins_csops.h
 --- ins_csops.h   4 Mar 2010 12:21:36 -   1.5
 +++ ins_csops.h   24 Dec 2014 12:34:49 -
 @@ -34,6 +34,6 @@
  #endif
  I've seen penguins that can type better than that.,
  Have you considered trying to match wits with a rutabaga?,
 -You speak an infinite deal of nothing,
 +You speak an infinite deal of nothing.,
  
  #endif /* _SUDO_INS_CSOPS_H */
 
 Index: ins_classic.h
 ===
 RCS file: /cvs/src/usr.bin/sudo/ins_classic.h,v
 retrieving revision 1.3
 diff -u -p -r1.3 ins_classic.h
 --- ins_classic.h 4 Mar 2010 12:21:36 -   1.3
 +++ ins_classic.h 24 Dec 2014 12:35:36 -
 @@ -21,7 +21,7 @@
   * Insults from the original sudo(8).
   */
  
 -Wrong!  You cheating scum!,
 +Wrong! You cheating scum!,
  #ifdef PC_INSULTS
  And you call yourself a Rocket Scientist!,
  #else
 
 Index: ins_goons.h
 ===
 RCS file: /cvs/src/usr.bin/sudo/ins_goons.h,v
 retrieving revision 1.3
 diff -u -p -r1.3 ins_goons.h
 --- ins_goons.h   4 Mar 2010 12:21:36 -   1.3
 +++ ins_goons.h   24 Dec 2014 12:38:56 -
 @@ -24,25 +24,25 @@
  You silly, twisted boy you.,
  He has fallen in the water!,
  We'll all be murdered in our beds!,
 -You can't come in. Our tiger has got flu,
 +You can't come in. Our tiger has got flu.,
  I don't wish to know that.,
  What, what, what, what, what, what, what, what, what, what?,
  You can't get the wood, you know.,
  You'll starve!,
  ... and it used to be so popular...,
 -Pauses for audience applause, not a sausage,
 +Pauses for audience applause, not a sausage.,
  Hold it up to the light --- not a brain in sight!,
  Have a gorilla...,
  There must be cure for it!,
  There's a lot of it about, you know.,
  You do that again and see what happens...,
 -Ying Tong Iddle I Po,
 +Ying Tong Iddle I Po.,
  Harm can come to a young lad like that!,
  And with that remarks folks, the case of the Crown vs yourself was 
 proven.,
  Speak English you fool --- there are no subtitles in this scene.,
  You gotta go ow!,
  I have been called worse.,
  It's only your word against mine.,
 -I think ... err ... I think ... I think I'll go home,
 +I think ... err ... I think ... I think I'll go home.,
  
  #endif /* _SUDO_INS_GOONS_H */




pgpYPOtIhUety.pgp
Description: PGP signature


[patch] siphash static functions

2015-01-16 Thread Fritjof Bornebusch
Hi tech@,

aren't these functions supposed to be static?

fritjof


Index: siphash.c
===
RCS file: /cvs/src/sys/crypto/siphash.c,v
retrieving revision 1.1
diff -u -p -r1.1 siphash.c
--- siphash.c   4 Nov 2014 03:01:14 -   1.1
+++ siphash.c   16 Jan 2015 10:41:37 -
@@ -48,8 +48,8 @@
 
 #include crypto/siphash.h
 
-void   SipHash_CRounds(SIPHASH_CTX *, int);
-void   SipHash_Rounds(SIPHASH_CTX *, int);
+static void SipHash_CRounds(SIPHASH_CTX *, int);
+static void SipHash_Rounds(SIPHASH_CTX *, int);
 
 void
 SipHash_Init(SIPHASH_CTX *ctx, const SIPHASH_KEY *key)
@@ -147,7 +147,7 @@ SipHash(const SIPHASH_KEY *key, int rc, 
 
 #define SIP_ROTL(x, b) ((x)  (b)) | ( (x)  (64 - (b)))
 
-void
+static void
 SipHash_Rounds(SIPHASH_CTX *ctx, int rounds)
 {
while (rounds--) {
@@ -171,7 +171,7 @@ SipHash_Rounds(SIPHASH_CTX *ctx, int rou
}
 }
 
-void
+static void
 SipHash_CRounds(SIPHASH_CTX *ctx, int rounds)
 {
u_int64_t m = lemtoh64((u_int64_t *)ctx-buf);


pgpoz5I_1ymPA.pgp
Description: PGP signature


[patch] remove atoi(3) from keynote

2015-01-16 Thread Fritjof Bornebusch
Hi tech@,

this diff removes the atoi(3) call from keynote(1).

fritjof


Index: keynote-keygen.c
===
RCS file: /cvs/src/lib/libkeynote/keynote-keygen.c,v
retrieving revision 1.21
diff -u -p -r1.21 keynote-keygen.c
--- keynote-keygen.c29 Jun 2004 11:35:56 -  1.21
+++ keynote-keygen.c16 Jan 2015 19:44:42 -
@@ -24,6 +24,7 @@
 
 #include ctype.h
 #include fcntl.h
+#include limits.h
 #include regex.h
 #include stdio.h
 #include stdlib.h
@@ -106,6 +107,7 @@ keynote_keygen(int argc, char *argv[])
 RSA *rsa;
 FILE *fp;
 char *algname;
+const char *errstr;
 
 if ((argc != 5)  (argc != 6)  (argc != 7))
 {
@@ -135,8 +137,8 @@ keynote_keygen(int argc, char *argv[])
 
 if (argc  5)
 {
-   begin = atoi(argv[5]);
-   if (begin = -1)
+   begin = strtonum(argv[5], 0, INT_MAX, errstr);
+   if (errstr)
{
fprintf(stderr, Erroneous value for print-offset parameter.\n);
exit(1);
@@ -145,8 +147,8 @@ keynote_keygen(int argc, char *argv[])
 
 if (argc  6)
 {
-   prlen = atoi(argv[6]);
-   if (prlen = 0)
+   prlen = strtonum(argv[6], 1, INT_MAX, errstr);
+   if (errstr)
{
fprintf(stderr, Erroneous value for print-length parameter.\n);
exit(1);
@@ -162,9 +164,9 @@ keynote_keygen(int argc, char *argv[])
 }
 
 alg = keynote_get_key_algorithm(algname, enc, ienc);
-len = atoi(argv[2]);
+len = strtonum(argv[2], 1, INT_MAX, errstr);
 
-if (len = 0)
+if (errstr)
 {
fprintf(stderr, Invalid specified keysize %d\n, len);
exit(1);
Index: keynote-sign.c
===
RCS file: /cvs/src/lib/libkeynote/keynote-sign.c,v
retrieving revision 1.16
diff -u -p -r1.16 keynote-sign.c
--- keynote-sign.c  29 Jun 2004 11:35:56 -  1.16
+++ keynote-sign.c  16 Jan 2015 19:44:46 -
@@ -23,6 +23,7 @@
 #include sys/stat.h
 
 #include ctype.h
+#include limits.h
 #include regex.h
 #include stdio.h
 #include stdlib.h
@@ -50,6 +51,7 @@ keynote_sign(int argc, char *argv[])
 char *buf, *buf2, *sig, *algname;
 int fd, flg = 0, buflen;
 struct stat sb;
+const char *errstr;
 
 if ((argc != 4) 
(argc != 5) 
@@ -65,8 +67,8 @@ keynote_sign(int argc, char *argv[])
 
 if (argc  4 + flg)
 {
-begin = atoi(argv[4 + flg]);
-if (begin = -1)
+   begin = strtonum(argv[4 + flg], 0, INT_MAX, errstr);
+if (errstr)
 {
 fprintf(stderr, Erroneous value for print-offset parameter.\n);
 exit(1);
@@ -75,8 +77,8 @@ keynote_sign(int argc, char *argv[])
 
 if (argc  5 + flg)
 {
-prlen = atoi(argv[5 + flg]);
-if (prlen = 0)
+   prlen = strtonum(argv[5 + flg], 1, INT_MAX, errstr);
+if (errstr)
 {
 fprintf(stderr, Erroneous value for print-length parameter.\n);
 exit(1);


pgp__Xagg7l1q.pgp
Description: PGP signature


[patch]sudo: punctuation fixes

2014-12-24 Thread Fritjof Bornebusch
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

2014-12-01 Thread Fritjof Bornebusch

fritjof


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.6
diff -u -p -r1.6 xmalloc.c
--- xmalloc.c   1 Dec 2014 21:58:46 -   1.6
+++ xmalloc.c   1 Dec 2014 23:59:50 -
@@ -60,7 +60,7 @@ xreallocarray(void *ptr, size_t nmemb, s
 
new_ptr = reallocarray(ptr, nmemb, size);
if (new_ptr == NULL)
-   errx(1, xrealloc: out of memory (new_size %zu bytes),
+   errx(1, xreallocarray: out of memory (new_size %zu bytes),
nmemb * size);
return new_ptr;
 }


pgpWKr1YLtM1Z.pgp
Description: PGP signature


[patch]rcs: comment typo

2014-11-29 Thread Fritjof Bornebusch
Hi tech,

it's NULL not NUL.

fritjof


Index: diff3.c
===
RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
retrieving revision 1.33
diff -u -p -r1.33 diff3.c
--- diff3.c 4 Mar 2012 04:05:15 -   1.33
+++ diff3.c 29 Nov 2014 13:15:51 -
@@ -450,7 +450,7 @@ ed_patch_lines(struct rcs_lines *dlines,
if (lp-l_len  2)
continue;
 
-   /* NUL-terminate line buffer for strtol() safety. */
+   /* NULL-terminate line buffer for strtol() safety. */
tmp = lp-l_line[lp-l_len - 1];
lp-l_line[lp-l_len - 1] = '\0';
 
Index: rcs.c
===
RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
retrieving revision 1.81
diff -u -p -r1.81 rcs.c
--- rcs.c   10 Oct 2014 08:15:25 -  1.81
+++ rcs.c   29 Nov 2014 13:16:40 -
@@ -799,7 +799,7 @@ rcs_patch_lines(struct rcs_lines *dlines
if (lp-l_len  2)
errx(1, line too short, RCS patch seems broken);
op = *(lp-l_line);
-   /* NUL-terminate line buffer for strtol() safety. */
+   /* NULL-terminate line buffer for strtol() safety. */
tmp = lp-l_line[lp-l_len - 1];
lp-l_line[lp-l_len - 1] = '\0';
lineno = (int)strtol((lp-l_line + 1), ep, 10);
@@ -1047,7 +1047,7 @@ rcs_delta_stats(struct rcs_delta *rdp, i
errx(1,
line too short, RCS patch seems broken);
op = *(lp-l_line);
-   /* NUL-terminate line buffer for strtol() safety. */
+   /* NULL-terminate line buffer for strtol() safety. */
tmp = lp-l_line[lp-l_len - 1];
lp-l_line[lp-l_len - 1] = '\0';
(void)strtol((lp-l_line + 1), ep, 10);


pgppOOEBjCeyo.pgp
Description: PGP signature


Re: [patch]rcs: comment typo

2014-11-29 Thread Fritjof Bornebusch
On Sat, Nov 29, 2014 at 05:27:00AM -0800, Claus Assmann wrote:
 On Sat, Nov 29, 2014, Fritjof Bornebusch wrote:
 
  it's NULL not NUL.
 
 Not in this case...
 
 NULL: is a pointer (usually 0)
 NUL: is a character ('\0')
 

Ahh I see, thank you.



pgpMkIwf4S_cz.pgp
Description: PGP signature


Re: [patch]rcs: comment typo

2014-11-29 Thread Fritjof Bornebusch
On Sat, Nov 29, 2014 at 04:53:28PM +0100, Otto Moerbeek wrote:
 On Sat, Nov 29, 2014 at 02:22:25PM +0100, Fritjof Bornebusch wrote:
 
  Hi tech,
  
  it's NULL not NUL.
 
 You're touching a big controversy here. Many developers say that NUL is
 the right term when rferring to chars and not pointers,


And what is the correct term when referring to '0'?
Or means '\0' and '0' the same.

 
   -Otto
 
  
  fritjof
  
  
  Index: diff3.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
  retrieving revision 1.33
  diff -u -p -r1.33 diff3.c
  --- diff3.c 4 Mar 2012 04:05:15 -   1.33
  +++ diff3.c 29 Nov 2014 13:15:51 -
  @@ -450,7 +450,7 @@ ed_patch_lines(struct rcs_lines *dlines,
  if (lp-l_len  2)
  continue;
   
  -   /* NUL-terminate line buffer for strtol() safety. */
  +   /* NULL-terminate line buffer for strtol() safety. */
  tmp = lp-l_line[lp-l_len - 1];
  lp-l_line[lp-l_len - 1] = '\0';
   
  Index: rcs.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
  retrieving revision 1.81
  diff -u -p -r1.81 rcs.c
  --- rcs.c   10 Oct 2014 08:15:25 -  1.81
  +++ rcs.c   29 Nov 2014 13:16:40 -
  @@ -799,7 +799,7 @@ rcs_patch_lines(struct rcs_lines *dlines
  if (lp-l_len  2)
  errx(1, line too short, RCS patch seems broken);
  op = *(lp-l_line);
  -   /* NUL-terminate line buffer for strtol() safety. */
  +   /* NULL-terminate line buffer for strtol() safety. */
  tmp = lp-l_line[lp-l_len - 1];
  lp-l_line[lp-l_len - 1] = '\0';
  lineno = (int)strtol((lp-l_line + 1), ep, 10);
  @@ -1047,7 +1047,7 @@ rcs_delta_stats(struct rcs_delta *rdp, i
  errx(1,
  line too short, RCS patch seems broken);
  op = *(lp-l_line);
  -   /* NUL-terminate line buffer for strtol() safety. */
  +   /* NULL-terminate line buffer for strtol() safety. */
  tmp = lp-l_line[lp-l_len - 1];
  lp-l_line[lp-l_len - 1] = '\0';
  (void)strtol((lp-l_line + 1), ep, 10);
 
 


pgpxlyH4t3nOW.pgp
Description: PGP signature


Re: [Patch]rcs: use rcsnum_cmp

2014-11-28 Thread Fritjof Bornebusch
On Fri, Nov 28, 2014 at 04:14:50PM +0100, Otto Moerbeek wrote:
 On Sun, Nov 23, 2014 at 05:19:16PM +0100, Fritjof Bornebusch wrote:
 
  Hi tech,
  
  like the XXX comment says, rcsnum_cmp() can be used instead of a *for* loop.
  The following shows the original behavior:
  
  $ co -r1.2 foo.txt,v
  foo.txt,v  --  foo.txt
  revision 1.2
  done
  
  $ co -r1.1 foo.txt,v 
  foo.txt,v  --  foo.txt
  revision 1.1
  done
  
  $ co foo.txt,v
  foo.txt,v  --  foo.txt
  revision 1.2
  done
  
  The following shows the changed behavior:
  
  $ co -r1.2 foo.txt,v 
  foo.txt,v  --  foo.txt
  revision 1.2
  done
  
  $ co -r1.1 foo.txt,v
  foo.txt,v  --  foo.txt
  revision 1.1
  done
  
  $ co foo.txt,v
  foo.txt,v  --  foo.txt
  revision 1.2
  done
  
  Could some verify that I didn't miss a test case.
 
 Hmm, your examples show no diffference. I think the intention is to
 not change the behaviour, right? 


Yes.
But as Tobias mentioned the behaviour will change anyway using my diff.
I'm checking out the official GnuRCS version regarding this right now.
 
   -Otto
 

fritjof

  
  fritjof
  
  Index: co.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/co.c,v
  retrieving revision 1.119
  diff -u -p -r1.119 co.c
  --- co.c10 Oct 2014 08:15:25 -  1.119
  +++ co.c23 Nov 2014 15:40:30 -
  @@ -265,18 +265,14 @@ checkout_rev(RCSFILE *file, RCSNUM *frev
  (void)fprintf(stderr,
  no revisions present; generating empty revision 0.0\n);
   
  -   /* XXX rcsnum_cmp()
  +   /*
   * Check out the latest revision if frev is greater than HEAD
   */
  if (file-rf_ndelta != 0) {
  -   for (i = 0; i  file-rf_head-rn_len; i++) {
  -   if (file-rf_head-rn_id[i]  frev-rn_id[i]) {
  -   frev = file-rf_head;
  -   break;
  -   }
  -   }
  +   if (rcsnum_cmp(file-rf_head, frev, 0) == 1)
  +   frev = file-rf_head;
  }
  -
  +   
  lcount = 0;
  TAILQ_FOREACH(lkp, (file-rf_locks), rl_list) {
  if (!strcmp(lkp-rl_name, lockname))
  
  
  Index: rcs.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
  retrieving revision 1.81
  diff -u -p -r1.81 rcs.c
  --- rcs.c   10 Oct 2014 08:15:25 -  1.81
  +++ rcs.c   23 Nov 2014 15:56:58 -
  @@ -905,16 +905,15 @@ rcs_getrev(RCSFILE *rfp, RCSNUM *frev)
  else
  rev = frev;
   
  -   /* XXX rcsnum_cmp() */
  -   for (i = 0; i  rfp-rf_head-rn_len; i++) {
  -   if (rfp-rf_head-rn_id[i]  rev-rn_id[i]) {
  -   rcs_errno = RCS_ERR_NOENT;
  -   return (NULL);
  -   }
  +   if(rcsnum_cmp(rfp-rf_head, rev, 0) == 1) {
  +   rcs_errno = RCS_ERR_NOENT;
  +   return (NULL);
  }
  -
  -   /* No matter what, we'll need everything parsed up until the description
  -   so go for it. */
  +   
  +   /*
  +* No matter what, we'll need everything parsed up until the description
  +* so go for it.
  +*/
  if (rcsparse_deltas(rfp, NULL))
  return (NULL);
   
 
 


pgpZD2UEfcOPl.pgp
Description: PGP signature


[Patch]rcs: use rcsnum_cmp

2014-11-23 Thread Fritjof Bornebusch
Hi tech,

like the XXX comment says, rcsnum_cmp() can be used instead of a *for* loop.
The following shows the original behavior:

$ co -r1.2 foo.txt,v
foo.txt,v  --  foo.txt
revision 1.2
done

$ co -r1.1 foo.txt,v 
foo.txt,v  --  foo.txt
revision 1.1
done

$ co foo.txt,v
foo.txt,v  --  foo.txt
revision 1.2
done

The following shows the changed behavior:

$ co -r1.2 foo.txt,v 
foo.txt,v  --  foo.txt
revision 1.2
done

$ co -r1.1 foo.txt,v
foo.txt,v  --  foo.txt
revision 1.1
done

$ co foo.txt,v
foo.txt,v  --  foo.txt
revision 1.2
done

Could some verify that I didn't miss a test case.

fritjof

Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.119
diff -u -p -r1.119 co.c
--- co.c10 Oct 2014 08:15:25 -  1.119
+++ co.c23 Nov 2014 15:40:30 -
@@ -265,18 +265,14 @@ checkout_rev(RCSFILE *file, RCSNUM *frev
(void)fprintf(stderr,
no revisions present; generating empty revision 0.0\n);
 
-   /* XXX rcsnum_cmp()
+   /*
 * Check out the latest revision if frev is greater than HEAD
 */
if (file-rf_ndelta != 0) {
-   for (i = 0; i  file-rf_head-rn_len; i++) {
-   if (file-rf_head-rn_id[i]  frev-rn_id[i]) {
-   frev = file-rf_head;
-   break;
-   }
-   }
+   if (rcsnum_cmp(file-rf_head, frev, 0) == 1)
+   frev = file-rf_head;
}
-
+   
lcount = 0;
TAILQ_FOREACH(lkp, (file-rf_locks), rl_list) {
if (!strcmp(lkp-rl_name, lockname))


Index: rcs.c
===
RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
retrieving revision 1.81
diff -u -p -r1.81 rcs.c
--- rcs.c   10 Oct 2014 08:15:25 -  1.81
+++ rcs.c   23 Nov 2014 15:56:58 -
@@ -905,16 +905,15 @@ rcs_getrev(RCSFILE *rfp, RCSNUM *frev)
else
rev = frev;
 
-   /* XXX rcsnum_cmp() */
-   for (i = 0; i  rfp-rf_head-rn_len; i++) {
-   if (rfp-rf_head-rn_id[i]  rev-rn_id[i]) {
-   rcs_errno = RCS_ERR_NOENT;
-   return (NULL);
-   }
+   if(rcsnum_cmp(rfp-rf_head, rev, 0) == 1) {
+   rcs_errno = RCS_ERR_NOENT;
+   return (NULL);
}
-
-   /* No matter what, we'll need everything parsed up until the description
-   so go for it. */
+   
+   /*
+* No matter what, we'll need everything parsed up until the description
+* so go for it.
+*/
if (rcsparse_deltas(rfp, NULL))
return (NULL);
 


pgp7P8LDyEFuk.pgp
Description: PGP signature


[PATCH]rcs: write usage function pointer always the same way

2014-11-20 Thread Fritjof Bornebusch
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

2014-10-13 Thread Fritjof Bornebusch
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

2014-10-07 Thread Fritjof Bornebusch
On Tue, Oct 07, 2014 at 03:10:44AM -0400, Daniel Dickman wrote:
 Fritjof, have you let the gnu rcs project know about the segfault?
 Maybe see how they choose to fix things and then follow their lead?


No, I have not.
I hope they follow the tech@ mailing list. :)
 
 
 On Mon, Oct 6, 2014 at 10:47 AM, Nicholas Marriott
 nicholas.marri...@gmail.com wrote:
 
  I think that GNU RCS segfaulting for -u -l is enough justification to do
  what we like, so a message (and last flag wins) like -L/-U would be fine
  with me.
 
  But if we want to do what they probably meant to happen then better to
  match -l -u like Fritjof's diff.
 
 
  On Fri, Oct 03, 2014 at 12:55:35PM +0200, Otto Moerbeek wrote:
   On Thu, Oct 02, 2014 at 12:56:10AM +0100, Nicholas Marriott wrote:
  
   
OTOH, check out what we do with rcs -L and -U...
  
   I kinda like that, because it tells you exactly what it is doing.
  
 -Otto
  
   
   
On Thu, Oct 02, 2014 at 12:54:13AM +0100, Nicholas Marriott wrote:
 Matching GNU RCS seems preferable to me but I don't feel strongly 
 about
 it.

 I wouldn't mention this in the man page, it hardly seems like 
 behaviour
 anyone should (or will need to) rely on.


 On Wed, Oct 01, 2014 at 07:41:52PM -0400, Daniel Dickman wrote:
  posix commands (like ls(1) for example) keep the last option when 
  mutually exclusive options are specified. does it make sense to 
  keep rcs consistent with that convention? also is a man page diff 
  needed?
 
 
   On Oct 1, 2014, at 7:17 PM, Nicholas Marriott 
   nicholas.marri...@gmail.com wrote:
  
   The existing behaviour isn't wildly useful, makes sense to me, ok 
   nicm
  
  
   On Wed, Oct 01, 2014 at 11:33:33PM +0200, Fritjof Bornebusch 
   wrote:
   Hi tech,
  
   the OpenRCS rcs command produces the following output if -l and 
   -u is
   used in the same command:
  
   $ rcs -l1.1 -u1.1 foo.txt
   RCS file: foo.txt,v
   1.1 locked
   1.1 unlocked
  
   $ rcs -u1.1 -l1.1 foo.txt
   RCS file: foo.txt,v
   1.1 locked
   1.1 unlocked
  
   I've looked at GnuRCS and it has another way to handle these 
   parameters
   (it seems the other BSDs use GnuRCS, too).
  
   Debian 7.5:
   $ rcs -l1.1 -u1.1 foo.txt
   RCS file: foo.txt,v
   rcs: foo.txt,v: no lock set on revision 1.1
   1.1 locked
  
   $ rcs -u1.1 -l1.1 foo.txt
   Segmentation fault
  
   Well, I think the Segmentation fault isn't that important :), 
   but GnuRCS
   does not lock and unlock a file by using the same command like 
   OpenRCS.
  
   I think the different implementations of RCS should share the 
   same
   behaviour:
  
   $ rcs -l1.1 -u1.1 foo.txt
   RCS file: foo.txt,v
   1.1 locked
   done
  
   $ rcs -u1.1 -l1.1 foo.txt
   RCS file: foo.txt,v
   1.1 unlocked
   done
  
   fritjof
  
  
   Index: rcsprog.c
   ===
   RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
   retrieving revision 1.151
   diff -u -p -r1.151 rcsprog.c
   --- rcsprog.c12 Jul 2011 21:00:32 -1.151
   +++ rcsprog.c3 Aug 2014 15:42:34 -
   @@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
  lkmode = RCS_LOCK_STRICT;
  break;
  case 'l':
   -/* XXX - Check with -u flag. */
   -lrev = rcs_optarg;
   -rcsflags |= RCSPROG_LFLAG;
   +if (!(rcsflags  RCSPROG_UFLAG)) {
   +lrev = rcs_optarg;
   +rcsflags |= RCSPROG_LFLAG;
   +}
  break;
  case 'm':
  if (logstr != NULL)
   @@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
  lkmode = RCS_LOCK_LOOSE;
  break;
  case 'u':
   -/* XXX - Check with -l flag. */
   -urev = rcs_optarg;
   -rcsflags |= RCSPROG_UFLAG;
   +if (!(rcsflags  RCSPROG_LFLAG)) {
   +urev = rcs_optarg;
   +rcsflags |= RCSPROG_UFLAG;
   +}
  break;
  case 'V':
  printf(%s\n, rcs_version);
  
 



Re: [patch]lock and unlock like GnuRCS

2014-10-07 Thread Fritjof Bornebusch
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

2014-10-07 Thread Fritjof Bornebusch
On Tue, Oct 07, 2014 at 03:11:28PM +0200, Otto Moerbeek wrote:
 On Tue, Oct 07, 2014 at 02:56:07PM +0200, Fritjof Bornebusch wrote:
 
  On Tue, Oct 07, 2014 at 09:34:33AM +0200, Otto Moerbeek wrote:
   On Tue, Oct 07, 2014 at 03:10:44AM -0400, Daniel Dickman wrote:
   
Fritjof, have you let the gnu rcs project know about the segfault?
Maybe see how they choose to fix things and then follow their lead?
   
   That will only slow things down. Do what -L -U does is better, imo.
  
  
  Otto, so you appreciate a diff more like this one?
 
 well, almost. I think it should be clear one flag and set one, not clear
 all and set one. 


Here is the new one.
 
   
 -Otto


  
  fritjof
  

fritjof

Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.152
diff -u -p -r1.152 rcsprog.c
--- rcsprog.c   2 Oct 2014 06:23:15 -   1.152
+++ rcsprog.c   7 Oct 2014 15:08:39 -
@@ -235,8 +235,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
-   /* XXX - Check with -u flag. */
+   if (rcsflags  RCSPROG_UFLAG)
+   warnx(-u overridden by -l);
lrev = rcs_optarg;
+   rcsflags = ~RCSPROG_UFLAG;
rcsflags |= RCSPROG_LFLAG;
break;
case 'm':
@@ -273,8 +275,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
-   /* XXX - Check with -l flag. */
+   if (rcsflags  RCSPROG_LFLAG)
+   warnx(-l overridden by -u);
urev = rcs_optarg;
+   rcsflags = ~RCSPROG_LFLAG;
rcsflags |= RCSPROG_UFLAG;
break;
case 'V':



Re: [Patch] use exit() directly in usage()

2014-10-01 Thread Fritjof Bornebusch
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,

 Hi,
 
 after usage() was called, there is no where you can go.


as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.

 fritjof
 

fritjof

Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 -  1.217
+++ ci.c1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static voidcheckin_parsekeyword(char *
 static int  checkin_update(struct checkin_params *);
 static int  checkin_revert(struct checkin_params *);
 
-void
+__dead void
 checkin_usage(void)
 {
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
  [-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
  [-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
  [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+   exit(1);
 }
 
 /*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-   exit(1);
}
 
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 -  1.117
+++ co.c1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-   exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-   exit (1);
}
 
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
 }
 
-void
+__dead void
 checkout_usage(void)
 {
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
  [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
  [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+   
+   exit(1);
 }
 
 /*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 -  1.7
+++ merge.c 1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
-   exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc  3) ? not enough : too many);
(usage)();
-   exit(D_ERROR);
}
 
for (; labels  3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
 }
 
-void
+__dead void
 merge_usage(void)
 {
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+   exit(D_ERROR);
 }
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c  28 Jul 2010 09:07:11 -  1.52
+++ rcsclean.c  1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-   exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -104,7 +102,6 @@ rcsclean_main(int argc, char **argv)
if ((dirp = opendir(.)) == NULL) {
warn(opendir);
(usage

Re: [Patch] use exit() directly in usage()

2014-10-01 Thread Fritjof Bornebusch
On Wed, Oct 01, 2014 at 06:41:25PM +0100, Nicholas Marriott wrote:
 Looks good but you have missed out ident.c and rcsprog.c


Ups, sorry.
 
 
 
 On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote:
  On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
  Hi,
  
   Hi,
   
   after usage() was called, there is no where you can go.
  
  
  as suggested by otto@ and @nicm, the usage() functions are marked as
  __dead.
  
   fritjof
   
  
  fritjof
 

fritjof

Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 -  1.217
+++ ci.c1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static voidcheckin_parsekeyword(char *
 static int  checkin_update(struct checkin_params *);
 static int  checkin_revert(struct checkin_params *);
 
-void
+__dead void
 checkin_usage(void)
 {
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
  [-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
  [-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
  [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+   exit(1);
 }
 
 /*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-   exit(1);
}
 
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 -  1.117
+++ co.c1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-   exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-   exit (1);
}
 
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
 }
 
-void
+__dead void
 checkout_usage(void)
 {
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
  [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
  [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+   
+   exit(1);
 }
 
 /*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 -  1.7
+++ merge.c 1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
-   exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc  3) ? not enough : too many);
(usage)();
-   exit(D_ERROR);
}
 
for (; labels  3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
 }
 
-void
+__dead void
 merge_usage(void)
 {
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+   exit(D_ERROR);
 }
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c  28 Jul 2010 09:07:11 -  1.52
+++ rcsclean.c  1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-   exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
(usage

[patch]lock and unlock like GnuRCS

2014-10-01 Thread Fritjof Bornebusch
Hi tech,

the OpenRCS rcs command produces the following output if -l and -u is 
used in the same command:

$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked

$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked

I've looked at GnuRCS and it has another way to handle these parameters 
(it seems the other BSDs use GnuRCS, too).

Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked

$ rcs -u1.1 -l1.1 foo.txt
Segmentation fault

Well, I think the Segmentation fault isn't that important :), but GnuRCS 
does not lock and unlock a file by using the same command like OpenRCS.

I think the different implementations of RCS should share the same 
behaviour:

$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
done

$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 unlocked
done

fritjof


Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c   12 Jul 2011 21:00:32 -  1.151
+++ rcsprog.c   3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
-   /* XXX - Check with -u flag. */
-   lrev = rcs_optarg;
-   rcsflags |= RCSPROG_LFLAG;
+   if (!(rcsflags  RCSPROG_UFLAG)) {
+   lrev = rcs_optarg;
+   rcsflags |= RCSPROG_LFLAG;
+   }
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
-   /* XXX - Check with -l flag. */
-   urev = rcs_optarg;
-   rcsflags |= RCSPROG_UFLAG;
+   if (!(rcsflags  RCSPROG_LFLAG)) {
+   urev = rcs_optarg;
+   rcsflags |= RCSPROG_UFLAG;
+   }
break;
case 'V':
printf(%s\n, rcs_version);



[Patch] avoid typecast

2014-09-27 Thread Fritjof Bornebusch
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

2014-09-26 Thread Fritjof Bornebusch
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

2014-09-24 Thread Fritjof Bornebusch
Hi,

I changed atoi to strtonum in order to avoid overflows.

fritjof



Index: rcstime.c
===
RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v
retrieving revision 1.4
diff -u -p -r1.4 rcstime.c
--- rcstime.c   29 Apr 2014 07:44:19 -  1.4
+++ rcstime.c   24 Sep 2014 15:06:42 -
@@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r
int tzone;
int pos;
char *h, *m;
+   const char *errstr;
struct tm *ltb;
time_t now;
 
@@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r
 
memcpy(tb, rdp-rd_date, sizeof(*tb));
 
-   tzone = atoi(h);
-   if ((tzone = 24) || (tzone = -24))
+   tzone = strtonum(h, -23, 23, errstr);
+   if (errstr)
errx(1, %s: not a known time zone, tz);
 
if (pos) {
@@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r
tb-tm_hour = 0;
 
if (m != NULL) {
-   tzone = atoi(m);
-   if (tzone = 60)
-   errx(1, %s: not a known time zone, tz);
+   tzone = strtonum(m, 0, 59, errstr);
+   if (errstr)
+   errx(1, %s: not a known minute, m);
 
if ((tb-tm_min + tzone) = 60) {
tb-tm_hour++;



Re: [PATCH] rcs: don't use lock and unlock in the same command

2014-08-16 Thread Fritjof Bornebusch
On Sun, Aug 03, 2014 at 06:00:45PM +0200, Fritjof Bornebusch wrote:
Ping?
 Hi tech,
 
 the OpenRCS rcs command produces the following output if -l and -u is used in 
 the
 same command:
 
 $ rcs -l1.1 -u1.1 foo.txt 
 RCS file: foo.txt,v
 1.1 locked
 1.1 unlocked
 
 $ rcs -u1.1 -l1.1 foo.txt 
 RCS file: foo.txt,v
 1.1 locked
 1.1 unlocked
 
 I've looked at GnuRCS and it has another way to handle these parameters (it 
 seems the other BSDs
 use GnuRCS, too).
 
 Debian 7.5:
 $ rcs -l1.1 -u1.1 foo.txt 
 RCS file: foo.txt,v
 rcs: foo.txt,v: no lock set on revision 1.1
 1.1 locked
 
 $ rcs -u1.1 -l1.1 foo.txt 
 Segmentation fault
 
 Well, I think the Segmentation fault isn't that important, but GnuRCS does 
 not lock and unlock
 a file by using the same command like OpenRCS.
 
 I think the different implementations of RCS should share the same behaviour, 
 so I changed OpenRCS:
 
 $ rcs -l1.1 -u1.1 foo.txt 
 RCS file: foo.txt,v
 1.1 locked
 done
 
 $ rcs -u1.1 -l1.1 foo.txt  
 RCS file: foo.txt,v
 1.1 unlocked
 done
 
 fritjof
 
 Index: rcsprog.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
 retrieving revision 1.151
 diff -u -p -r1.151 rcsprog.c
 --- rcsprog.c 12 Jul 2011 21:00:32 -  1.151
 +++ rcsprog.c 3 Aug 2014 15:42:34 -
 @@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
   lkmode = RCS_LOCK_STRICT;
   break;
   case 'l':
 - /* XXX - Check with -u flag. */
 - lrev = rcs_optarg;
 - rcsflags |= RCSPROG_LFLAG;
 + if (!(rcsflags  RCSPROG_UFLAG)) {
 + lrev = rcs_optarg;
 + rcsflags |= RCSPROG_LFLAG;
 + }
   break;
   case 'm':
   if (logstr != NULL)
 @@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
   lkmode = RCS_LOCK_LOOSE;
   break;
   case 'u':
 - /* XXX - Check with -l flag. */
 - urev = rcs_optarg;
 - rcsflags |= RCSPROG_UFLAG;
 + if (!(rcsflags  RCSPROG_LFLAG)) {
 + urev = rcs_optarg;
 + rcsflags |= RCSPROG_UFLAG;
 + }
   break;
   case 'V':
   printf(%s\n, rcs_version);
 



Re: [PATCH]unnecessary typecast in rcs xmalloc

2014-08-16 Thread Fritjof Bornebusch
On Wed, Jul 30, 2014 at 10:23:00PM +0200, Fritjof Bornebusch wrote:
Ping?
 Hi tech,
 
 there is an unnecessary typecast in xmalloc.c of rcs.
 
 fritjof
 
 
 Index: xmalloc.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
 retrieving revision 1.4
 diff -u -p -r1.4 xmalloc.c
 --- xmalloc.c 7 Jun 2009 08:39:13 -   1.4
 +++ xmalloc.c 20 Jun 2014 10:21:37 -
 @@ -32,8 +32,8 @@ xmalloc(size_t size)
   ptr = malloc(size);
   if (ptr == NULL)
   errx(1,
 - xmalloc: out of memory (allocating %lu bytes),
 - (u_long) size);
 + xmalloc: out of memory (allocating %zu bytes),
 + size);
   return ptr;
  }
  
 @@ -48,8 +48,8 @@ xcalloc(size_t nmemb, size_t size)
   errx(1, xcalloc: nmemb * size  SIZE_MAX);
   ptr = calloc(nmemb, size);
   if (ptr == NULL)
 - errx(1, xcalloc: out of memory (allocating %lu bytes),
 - (u_long)(size * nmemb));
 + errx(1, xcalloc: out of memory (allocating %zu bytes),
 + (size * nmemb));
   return ptr;
  }
  
 @@ -68,8 +68,8 @@ xrealloc(void *ptr, size_t nmemb, size_t
   else
   new_ptr = realloc(ptr, new_size);
   if (new_ptr == NULL)
 - errx(1, xrealloc: out of memory (new_size %lu bytes),
 - (u_long) new_size);
 + errx(1, xrealloc: out of memory (new_size %zu bytes),
 + new_size);
   return new_ptr;
  }
  
 



Re: [PATCH]unused NULL check before calling free

2014-08-16 Thread Fritjof Bornebusch
On Sat, Aug 02, 2014 at 10:35:43PM +0200, Fritjof Bornebusch wrote:
Ping?
 On Fri, Aug 01, 2014 at 08:03:58AM -0400, Ted Unangst wrote:
  Half true. :)
  
  The behavior is intended. I don't really know why they care about
  freeing null, but the intention is clearly to check for it; otherwise
  they would just call free() in the first place. (actually, i think the
  rationale is that to assure portability, you need a strict version of
  free that matches the broken version found elsewhere or you will
  accidentally do something not portable. under that assumption, and
  assuming we don't care about those platforms, the correct fix is to
  delete xfree entirely.)
  
  The X option, though, to malloc.conf doesn't check NULL to free. It
  just makes malloc abort instead of returning null. I don't think it's
  a useful option either way. Don't use it.
 
 Based on Teds suggestion, this diff deletes xfree() entirely.
 
 fritjof
 
 Index: xmalloc.h
 ===
 RCS file: /cvs/src/usr.bin/rcs/xmalloc.h,v
 retrieving revision 1.1
 diff -u -p -r1.1 xmalloc.h
 --- xmalloc.h 26 Apr 2006 02:55:13 -  1.1
 +++ xmalloc.h 2 Aug 2014 20:09:55 -
 @@ -22,7 +22,6 @@
  void *xmalloc(size_t);
  void *xcalloc(size_t, size_t);
  void *xrealloc(void *, size_t, size_t);
 -void xfree(void *);
  char *xstrdup(const char *);
  int   xasprintf(char **, const char *, ...)
  __attribute__((__format__ (printf, 2, 3)))
 Index: xmalloc.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
 retrieving revision 1.4
 diff -u -p -r1.4 xmalloc.c
 --- xmalloc.c 7 Jun 2009 08:39:13 -   1.4
 +++ xmalloc.c 2 Aug 2014 20:10:08 -
 @@ -73,14 +73,6 @@ xrealloc(void *ptr, size_t nmemb, size_t
   return new_ptr;
  }
  
 -void
 -xfree(void *ptr)
 -{
 - if (ptr == NULL)
 - errx(1, xfree: NULL pointer given as argument);
 - free(ptr);
 -}
 -
  char *
  xstrdup(const char *str)
  {
 Index: ci.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/ci.c,v
 retrieving revision 1.217
 diff -u -p -r1.217 ci.c
 --- ci.c  19 May 2014 19:42:24 -  1.217
 +++ ci.c  2 Aug 2014 20:12:30 -
 @@ -208,8 +208,7 @@ checkin_main(int argc, char **argv)
   printf(%s\n, rcs_version);
   exit(0);
   case 'w':
 - if (pb.author != NULL)
 - xfree(pb.author);
 + free(pb.author);
   pb.author = xstrdup(rcs_optarg);
   break;
   case 'x':
 @@ -376,10 +375,9 @@ out:
   buf_free(b2);
   if (b3 != NULL)
   buf_free(b3);
 - if (path1 != NULL)
 - xfree(path1);
 - if (path2 != NULL)
 - xfree(path2);
 + 
 + free(path1);
 + free(path2);
  
   return (NULL);
  }
 @@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb
   fprintf(stderr,
   reuse log message of previous file? [yn](y): );
   if (rcs_yesno('y') != 'y') {
 - xfree(pb-rcs_msg);
 + free(pb-rcs_msg);
   pb-rcs_msg = NULL;
   }
   }
 @@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb
   pb-username, pb-author, NULL, NULL);
  
   if ((pb-flags  INTERACTIVE)  (pb-rcs_msg[0] == '\0')) {
 - xfree(pb-rcs_msg); /* free empty log message */
 + free(pb-rcs_msg);  /* free empty log message */
   pb-rcs_msg = NULL;
   }
  
 @@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC
   (void)xasprintf(datestring, %s %s, tokens[3], tokens[4]);
   if ((*date = date_parse(datestring)) == -1)
   errx(1, could not parse date);
 - xfree(datestring);
 + free(datestring);
  
   if (i  6)
   break;
 - if (*author != NULL)
 - xfree(*author);
 + free(*author);
   *author = xstrdup(tokens[5]);
  
   if (i  7)
   break;
 - if (*state != NULL)
 - xfree(*state);
 + free(*state);
   *state = xstrdup(tokens[6]);
   break;
   case KW_TYPE_AUTHOR:
   if (i  2)
   break;
 - if (*author != NULL)
 - xfree(*author);
 + free(*author);
   *author = xstrdup(tokens[1]);
   break;
   case KW_TYPE_DATE:
 @@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC
   (void)xasprintf(datestring, %s %s, tokens[1], tokens[2

Re: [PATCH] Better overflow handling in rcstime.c

2014-08-16 Thread Fritjof Bornebusch
On Wed, Jul 30, 2014 at 10:19:19PM +0200, Fritjof Bornebusch wrote:
Ping?
 Hi tech,
 
 remove the atoi calls, in order to avoid overflows.
 
 fritjof
 
 
 Index: rcstime.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v
 retrieving revision 1.4
 diff -u -p -r1.4 rcstime.c
 --- rcstime.c 29 Apr 2014 07:44:19 -  1.4
 +++ rcstime.c 30 Jun 2014 12:59:42 -
 @@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r
   int tzone;
   int pos;
   char *h, *m;
 + const char *errstr = NULL;
   struct tm *ltb;
   time_t now;
  
 @@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r
  
   memcpy(tb, rdp-rd_date, sizeof(*tb));
  
 - tzone = atoi(h);
 - if ((tzone = 24) || (tzone = -24))
 + tzone = (int)strtonum(h, -23, 23, errstr);
 + if (errstr)
   errx(1, %s: not a known time zone, tz);
  
   if (pos) {
 @@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r
   tb-tm_hour = 0;
  
   if (m != NULL) {
 - tzone = atoi(m);
 - if (tzone = 60)
 - errx(1, %s: not a known time zone, tz);
 + tzone = (int)strtonum(m, 0, 59, errstr);
 + if (errstr)
 + errx(1, %s: not a known minute, m);
  
   if ((tb-tm_min + tzone) = 60) {
   tb-tm_hour++;
 



Re: [PATCH]delete xfree() from sndiod

2014-08-16 Thread Fritjof Bornebusch
On Sun, Aug 03, 2014 at 02:56:25PM +0200, Fritjof Bornebusch wrote:
Ping?
 Hi tech,
 
 during my search after other xfree() implementations, I saw that xfree() in 
 sndiod is just a wrapper for free()
 without any other conditions, like NULL check.
 
 fritjof
 
 
 Index: abuf.c
 ===
 RCS file: /cvs/src/usr.bin/sndiod/abuf.c,v
 retrieving revision 1.2
 diff -u -p -r1.2 abuf.c
 --- abuf.c7 Dec 2012 08:04:58 -   1.2
 +++ abuf.c3 Aug 2014 12:37:19 -
 @@ -62,7 +62,7 @@ abuf_done(struct abuf *buf)
   }
   }
  #endif
 - xfree(buf-data);
 + free(buf-data);
   buf-data = (void *)0xdeadbeef;
  }
  
 Index: dev.c
 ===
 RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
 retrieving revision 1.17
 diff -u -p -r1.17 dev.c
 --- dev.c 2 Jun 2014 07:51:25 -   1.17
 +++ dev.c 3 Aug 2014 12:38:52 -
 @@ -15,6 +15,7 @@
   * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
   */
  #include stdio.h
 +#include stdlib.h
  #include string.h
  
  #include abuf.h
 @@ -838,10 +839,8 @@ dev_cycle(struct dev *d)
*/
   s-pstate = SLOT_INIT;
   abuf_done(s-mix.buf);
 - if (s-mix.decbuf)
 - xfree(s-mix.decbuf);
 - if (s-mix.resampbuf)
 - xfree(s-mix.resampbuf);
 + free(s-mix.decbuf);
 + free(s-mix.resampbuf);
   s-ops-eof(s-arg);
   *ps = s-next;
   dev_mix_adjvol(d);
 @@ -1143,14 +1142,12 @@ dev_close(struct dev *d)
   d-slot_list = NULL;
   dev_sio_close(d);
   if (d-mode  MODE_PLAY) {
 - if (d-encbuf != NULL)
 - xfree(d-encbuf);
 - xfree(d-pbuf);
 + free(d-encbuf);
 + free(d-pbuf);
   }
   if (d-mode  MODE_REC) {
 - if (d-decbuf != NULL)
 - xfree(d-decbuf);
 - xfree(d-rbuf);
 + free(d-decbuf);
 + free(d-rbuf);
   }
  }
  
 @@ -1256,7 +1253,7 @@ dev_del(struct dev *d)
   }
   midi_del(d-midi);
   *p = d-next;
 - xfree(d);
 + free(d);
  }
  
  unsigned int
 @@ -1829,16 +1826,12 @@ slot_detach(struct slot *s)
   }   
   *ps = s-next;
   if (s-mode  MODE_RECMASK) {
 - if (s-sub.encbuf)
 - xfree(s-sub.encbuf);
 - if (s-sub.resampbuf)
 - xfree(s-sub.resampbuf);
 + free(s-sub.encbuf);
 + free(s-sub.resampbuf);
   }
   if (s-mode  MODE_PLAY) {
 - if (s-mix.decbuf)
 - xfree(s-mix.decbuf);
 - if (s-mix.resampbuf)
 - xfree(s-mix.resampbuf);
 + free(s-mix.decbuf);
 + free(s-mix.resampbuf);
   dev_mix_adjvol(s-dev);
   }
  }
 Index: file.c
 ===
 RCS file: /cvs/src/usr.bin/sndiod/file.c,v
 retrieving revision 1.5
 diff -u -p -r1.5 file.c
 --- file.c17 Mar 2014 17:17:01 -  1.5
 +++ file.c3 Aug 2014 12:40:30 -
 @@ -294,7 +294,7 @@ file_poll(void)
   while ((f = *pf) != NULL) {
   if (f-state == FILE_ZOMB) {
   *pf = f-next;
 - xfree(f);
 + free(f);
   } else
   pf = f-next;
   }
 Index: listen.c
 ===
 RCS file: /cvs/src/usr.bin/sndiod/listen.c,v
 retrieving revision 1.2
 diff -u -p -r1.2 listen.c
 --- listen.c  13 Mar 2013 08:28:33 -  1.2
 +++ listen.c  3 Aug 2014 12:40:55 -
 @@ -70,13 +70,13 @@ listen_close(struct listen *f)
   }
   *pf = f-next;
  
 - if (f-path != NULL) {
 + if (f-path != NULL)
   unlink(f-path);
 - xfree(f-path);
 - }
 + 
 + free(f-path);
   file_del(f-file);
   close(f-fd);
 - xfree(f);
 + free(f);
  }
  
  void
 Index: midi.c
 ===
 RCS file: /cvs/src/usr.bin/sndiod/midi.c,v
 retrieving revision 1.10
 diff -u -p -r1.10 midi.c
 --- midi.c28 Sep 2013 18:49:32 -  1.10
 +++ midi.c3 Aug 2014 12:42:08 -
 @@ -461,7 +461,7 @@ port_del(struct port *c)
  #endif
   }
   *p = c-next;
 - xfree(c);
 + free(c);
  }
  
  int
 Index: opt.c
 ===
 RCS file: /cvs/src/usr.bin/sndiod/opt.c,v
 retrieving revision 1.2
 diff -u -p -r1.2 opt.c
 --- opt.c 7 Dec 2012 08:04:58 -   1.2
 +++ opt.c 3 Aug 2014 12:43:52 -
 @@ -136,5 +136,5 @@ opt_del(struct opt *o)
  #endif
   }
   *po = o

[PATCH]delete xfree() from sndiod

2014-08-03 Thread Fritjof Bornebusch
Hi tech,

during my search after other xfree() implementations, I saw that xfree() in 
sndiod is just a wrapper for free()
without any other conditions, like NULL check.

fritjof


Index: abuf.c
===
RCS file: /cvs/src/usr.bin/sndiod/abuf.c,v
retrieving revision 1.2
diff -u -p -r1.2 abuf.c
--- abuf.c  7 Dec 2012 08:04:58 -   1.2
+++ abuf.c  3 Aug 2014 12:37:19 -
@@ -62,7 +62,7 @@ abuf_done(struct abuf *buf)
}
}
 #endif
-   xfree(buf-data);
+   free(buf-data);
buf-data = (void *)0xdeadbeef;
 }
 
Index: dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.17
diff -u -p -r1.17 dev.c
--- dev.c   2 Jun 2014 07:51:25 -   1.17
+++ dev.c   3 Aug 2014 12:38:52 -
@@ -15,6 +15,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 #include stdio.h
+#include stdlib.h
 #include string.h
 
 #include abuf.h
@@ -838,10 +839,8 @@ dev_cycle(struct dev *d)
 */
s-pstate = SLOT_INIT;
abuf_done(s-mix.buf);
-   if (s-mix.decbuf)
-   xfree(s-mix.decbuf);
-   if (s-mix.resampbuf)
-   xfree(s-mix.resampbuf);
+   free(s-mix.decbuf);
+   free(s-mix.resampbuf);
s-ops-eof(s-arg);
*ps = s-next;
dev_mix_adjvol(d);
@@ -1143,14 +1142,12 @@ dev_close(struct dev *d)
d-slot_list = NULL;
dev_sio_close(d);
if (d-mode  MODE_PLAY) {
-   if (d-encbuf != NULL)
-   xfree(d-encbuf);
-   xfree(d-pbuf);
+   free(d-encbuf);
+   free(d-pbuf);
}
if (d-mode  MODE_REC) {
-   if (d-decbuf != NULL)
-   xfree(d-decbuf);
-   xfree(d-rbuf);
+   free(d-decbuf);
+   free(d-rbuf);
}
 }
 
@@ -1256,7 +1253,7 @@ dev_del(struct dev *d)
}
midi_del(d-midi);
*p = d-next;
-   xfree(d);
+   free(d);
 }
 
 unsigned int
@@ -1829,16 +1826,12 @@ slot_detach(struct slot *s)
}   
*ps = s-next;
if (s-mode  MODE_RECMASK) {
-   if (s-sub.encbuf)
-   xfree(s-sub.encbuf);
-   if (s-sub.resampbuf)
-   xfree(s-sub.resampbuf);
+   free(s-sub.encbuf);
+   free(s-sub.resampbuf);
}
if (s-mode  MODE_PLAY) {
-   if (s-mix.decbuf)
-   xfree(s-mix.decbuf);
-   if (s-mix.resampbuf)
-   xfree(s-mix.resampbuf);
+   free(s-mix.decbuf);
+   free(s-mix.resampbuf);
dev_mix_adjvol(s-dev);
}
 }
Index: file.c
===
RCS file: /cvs/src/usr.bin/sndiod/file.c,v
retrieving revision 1.5
diff -u -p -r1.5 file.c
--- file.c  17 Mar 2014 17:17:01 -  1.5
+++ file.c  3 Aug 2014 12:40:30 -
@@ -294,7 +294,7 @@ file_poll(void)
while ((f = *pf) != NULL) {
if (f-state == FILE_ZOMB) {
*pf = f-next;
-   xfree(f);
+   free(f);
} else
pf = f-next;
}
Index: listen.c
===
RCS file: /cvs/src/usr.bin/sndiod/listen.c,v
retrieving revision 1.2
diff -u -p -r1.2 listen.c
--- listen.c13 Mar 2013 08:28:33 -  1.2
+++ listen.c3 Aug 2014 12:40:55 -
@@ -70,13 +70,13 @@ listen_close(struct listen *f)
}
*pf = f-next;
 
-   if (f-path != NULL) {
+   if (f-path != NULL)
unlink(f-path);
-   xfree(f-path);
-   }
+   
+   free(f-path);
file_del(f-file);
close(f-fd);
-   xfree(f);
+   free(f);
 }
 
 void
Index: midi.c
===
RCS file: /cvs/src/usr.bin/sndiod/midi.c,v
retrieving revision 1.10
diff -u -p -r1.10 midi.c
--- midi.c  28 Sep 2013 18:49:32 -  1.10
+++ midi.c  3 Aug 2014 12:42:08 -
@@ -461,7 +461,7 @@ port_del(struct port *c)
 #endif
}
*p = c-next;
-   xfree(c);
+   free(c);
 }
 
 int
Index: opt.c
===
RCS file: /cvs/src/usr.bin/sndiod/opt.c,v
retrieving revision 1.2
diff -u -p -r1.2 opt.c
--- opt.c   7 Dec 2012 08:04:58 -   1.2
+++ opt.c   3 Aug 2014 12:43:52 -
@@ -136,5 +136,5 @@ opt_del(struct opt *o)
 #endif
}
*po = o-next;
-   xfree(o);
+   free(o);
 }
Index: 

[PATCH] rcs: don't use lock and unlock in the same command

2014-08-03 Thread Fritjof Bornebusch
Hi tech,

the OpenRCS rcs command produces the following output if -l and -u is used in 
the
same command:

$ rcs -l1.1 -u1.1 foo.txt 
RCS file: foo.txt,v
1.1 locked
1.1 unlocked

$ rcs -u1.1 -l1.1 foo.txt 
RCS file: foo.txt,v
1.1 locked
1.1 unlocked

I've looked at GnuRCS and it has another way to handle these parameters (it 
seems the other BSDs
use GnuRCS, too).

Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt 
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked

$ rcs -u1.1 -l1.1 foo.txt 
Segmentation fault

Well, I think the Segmentation fault isn't that important, but GnuRCS does 
not lock and unlock
a file by using the same command like OpenRCS.

I think the different implementations of RCS should share the same behaviour, 
so I changed OpenRCS:

$ rcs -l1.1 -u1.1 foo.txt 
RCS file: foo.txt,v
1.1 locked
done

$ rcs -u1.1 -l1.1 foo.txt  
RCS file: foo.txt,v
1.1 unlocked
done

fritjof

Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c   12 Jul 2011 21:00:32 -  1.151
+++ rcsprog.c   3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
-   /* XXX - Check with -u flag. */
-   lrev = rcs_optarg;
-   rcsflags |= RCSPROG_LFLAG;
+   if (!(rcsflags  RCSPROG_UFLAG)) {
+   lrev = rcs_optarg;
+   rcsflags |= RCSPROG_LFLAG;
+   }
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
-   /* XXX - Check with -l flag. */
-   urev = rcs_optarg;
-   rcsflags |= RCSPROG_UFLAG;
+   if (!(rcsflags  RCSPROG_LFLAG)) {
+   urev = rcs_optarg;
+   rcsflags |= RCSPROG_UFLAG;
+   }
break;
case 'V':
printf(%s\n, rcs_version);



Re: [PATCH]unused NULL check before calling free

2014-08-02 Thread Fritjof Bornebusch
On Fri, Aug 01, 2014 at 08:03:58AM -0400, Ted Unangst wrote:
 Half true. :)
 
 The behavior is intended. I don't really know why they care about
 freeing null, but the intention is clearly to check for it; otherwise
 they would just call free() in the first place. (actually, i think the
 rationale is that to assure portability, you need a strict version of
 free that matches the broken version found elsewhere or you will
 accidentally do something not portable. under that assumption, and
 assuming we don't care about those platforms, the correct fix is to
 delete xfree entirely.)
 
 The X option, though, to malloc.conf doesn't check NULL to free. It
 just makes malloc abort instead of returning null. I don't think it's
 a useful option either way. Don't use it.

Based on Teds suggestion, this diff deletes xfree() entirely.

fritjof

Index: xmalloc.h
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.h,v
retrieving revision 1.1
diff -u -p -r1.1 xmalloc.h
--- xmalloc.h   26 Apr 2006 02:55:13 -  1.1
+++ xmalloc.h   2 Aug 2014 20:09:55 -
@@ -22,7 +22,6 @@
 void   *xmalloc(size_t);
 void   *xcalloc(size_t, size_t);
 void   *xrealloc(void *, size_t, size_t);
-void xfree(void *);
 char   *xstrdup(const char *);
 int xasprintf(char **, const char *, ...)
 __attribute__((__format__ (printf, 2, 3)))
Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.4
diff -u -p -r1.4 xmalloc.c
--- xmalloc.c   7 Jun 2009 08:39:13 -   1.4
+++ xmalloc.c   2 Aug 2014 20:10:08 -
@@ -73,14 +73,6 @@ xrealloc(void *ptr, size_t nmemb, size_t
return new_ptr;
 }
 
-void
-xfree(void *ptr)
-{
-   if (ptr == NULL)
-   errx(1, xfree: NULL pointer given as argument);
-   free(ptr);
-}
-
 char *
 xstrdup(const char *str)
 {
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 -  1.217
+++ ci.c2 Aug 2014 20:12:30 -
@@ -208,8 +208,7 @@ checkin_main(int argc, char **argv)
printf(%s\n, rcs_version);
exit(0);
case 'w':
-   if (pb.author != NULL)
-   xfree(pb.author);
+   free(pb.author);
pb.author = xstrdup(rcs_optarg);
break;
case 'x':
@@ -376,10 +375,9 @@ out:
buf_free(b2);
if (b3 != NULL)
buf_free(b3);
-   if (path1 != NULL)
-   xfree(path1);
-   if (path2 != NULL)
-   xfree(path2);
+   
+   free(path1);
+   free(path2);
 
return (NULL);
 }
@@ -511,7 +509,7 @@ checkin_update(struct checkin_params *pb
fprintf(stderr,
reuse log message of previous file? [yn](y): );
if (rcs_yesno('y') != 'y') {
-   xfree(pb-rcs_msg);
+   free(pb-rcs_msg);
pb-rcs_msg = NULL;
}
}
@@ -584,7 +582,7 @@ checkin_update(struct checkin_params *pb
pb-username, pb-author, NULL, NULL);
 
if ((pb-flags  INTERACTIVE)  (pb-rcs_msg[0] == '\0')) {
-   xfree(pb-rcs_msg); /* free empty log message */
+   free(pb-rcs_msg);  /* free empty log message */
pb-rcs_msg = NULL;
}
 
@@ -988,25 +986,22 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[3], tokens[4]);
if ((*date = date_parse(datestring)) == -1)
errx(1, could not parse date);
-   xfree(datestring);
+   free(datestring);
 
if (i  6)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[5]);
 
if (i  7)
break;
-   if (*state != NULL)
-   xfree(*state);
+   free(*state);
*state = xstrdup(tokens[6]);
break;
case KW_TYPE_AUTHOR:
if (i  2)
break;
-   if (*author != NULL)
-   xfree(*author);
+   free(*author);
*author = xstrdup(tokens[1]);
break;
case KW_TYPE_DATE:
@@ -1015,13 +1010,12 @@ checkin_parsekeyword(char *keystring, RC
(void)xasprintf(datestring, %s %s, tokens[1], tokens[2]);
if ((*date = date_parse(datestring)) == -1)

Re: [PATCH] Better overflow handling in rcstime.c

2014-08-02 Thread Fritjof Bornebusch
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

2014-07-31 Thread Fritjof Bornebusch
On Wed, Jul 30, 2014 at 07:37:29PM -0700, patrick keshishian wrote:
 On Wed, Jul 30, 2014 at 10:14:54PM +0200, Fritjof Bornebusch wrote:
  Hi tech,
  
  there is an unnecessary NULL check before calling free.
  
  fritjof
  
  Index: xmalloc.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
  retrieving revision 1.4
  diff -u -p -r1.4 xmalloc.c
  --- xmalloc.c   7 Jun 2009 08:39:13 -   1.4
  +++ xmalloc.c   31 May 2014 19:19:18 -
  @@ -76,8 +76,6 @@ xrealloc(void *ptr, size_t nmemb, size_t
   void
   xfree(void *ptr)
   {
  -   if (ptr == NULL)
  -   errx(1, xfree: NULL pointer given as argument);
  free(ptr);
   }
 
 
 Going by this context, a quick grep in src/usr.bin/rcs, and
 especially the error message, the NULL check's purpose is to
 catach this condition.
 
 The code you claim to correct:
 
  there is an unnecessary NULL check before calling free.
 
 would have been of the form:
 
   if (ptr != NULL)-or-if (ptr == NULL)
   free(ptr);  return;
 
That is true, but free() can handle NULL itself:

...
/* This is legal. */
if (ptr == NULL)
return;


That's why there is no need, to use a condition like:
if(ptr == NULL)
errx(1, xfree: NULL pointer given as argument);

 --patrick
 
fritjof



Re: [PATCH]unused NULL check before calling free

2014-07-31 Thread Fritjof Bornebusch
On Thu, Jul 31, 2014 at 10:32:07AM -0400, sven falempin wrote:
 On Thu, Jul 31, 2014 at 6:39 AM, Fritjof Bornebusch frit...@alokat.org 
 wrote:
  On Wed, Jul 30, 2014 at 07:37:29PM -0700, patrick keshishian wrote:
  On Wed, Jul 30, 2014 at 10:14:54PM +0200, Fritjof Bornebusch wrote:
   Hi tech,
  
   there is an unnecessary NULL check before calling free.
  
   fritjof
  
   Index: xmalloc.c
   ===
   RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
   retrieving revision 1.4
   diff -u -p -r1.4 xmalloc.c
   --- xmalloc.c   7 Jun 2009 08:39:13 -   1.4
   +++ xmalloc.c   31 May 2014 19:19:18 -
   @@ -76,8 +76,6 @@ xrealloc(void *ptr, size_t nmemb, size_t
void
xfree(void *ptr)
{
   -   if (ptr == NULL)
   -   errx(1, xfree: NULL pointer given as argument);
   free(ptr);
}
 
 
  Going by this context, a quick grep in src/usr.bin/rcs, and
  especially the error message, the NULL check's purpose is to
  catach this condition.
 
  The code you claim to correct:
 
   there is an unnecessary NULL check before calling free.
 
  would have been of the form:
 
if (ptr != NULL)-or-if (ptr == NULL)
free(ptr);  return;
 
  That is true, but free() can handle NULL itself:
 
  ...
  /* This is legal. */
  if (ptr == NULL)
  return;
  
 
  That's why there is no need, to use a condition like:
  if(ptr == NULL)
  errx(1, xfree: NULL pointer given as argument);
 
  --patrick
 
  fritjof
 
 
 For educational purpose i guess humanity need a free(NULL) is valid T shirt,
 i learned it in the mailing list around a year ago.
 
 Maybe the back would be if not, patch your OS, or better use OpenBSD
 
That would be very cool!

 -- 
 -
 () ascii ribbon campaign - against html e-mail
 /\
 



[PATCH]unused NULL check before calling free

2014-07-30 Thread Fritjof Bornebusch
Hi tech,

there is an unnecessary NULL check before calling free.

fritjof

Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.4
diff -u -p -r1.4 xmalloc.c
--- xmalloc.c   7 Jun 2009 08:39:13 -   1.4
+++ xmalloc.c   31 May 2014 19:19:18 -
@@ -76,8 +76,6 @@ xrealloc(void *ptr, size_t nmemb, size_t
 void
 xfree(void *ptr)
 {
-   if (ptr == NULL)
-   errx(1, xfree: NULL pointer given as argument);
free(ptr);
 }



[PATCH] Better overflow handling in rcstime.c

2014-07-30 Thread Fritjof Bornebusch
Hi tech,

remove the atoi calls, in order to avoid overflows.

fritjof


Index: rcstime.c
===
RCS file: /cvs/src/usr.bin/rcs/rcstime.c,v
retrieving revision 1.4
diff -u -p -r1.4 rcstime.c
--- rcstime.c   29 Apr 2014 07:44:19 -  1.4
+++ rcstime.c   30 Jun 2014 12:59:42 -
@@ -36,6 +36,7 @@ rcs_set_tz(char *tz, struct rcs_delta *r
int tzone;
int pos;
char *h, *m;
+   const char *errstr = NULL;
struct tm *ltb;
time_t now;
 
@@ -62,8 +63,8 @@ rcs_set_tz(char *tz, struct rcs_delta *r
 
memcpy(tb, rdp-rd_date, sizeof(*tb));
 
-   tzone = atoi(h);
-   if ((tzone = 24) || (tzone = -24))
+   tzone = (int)strtonum(h, -23, 23, errstr);
+   if (errstr)
errx(1, %s: not a known time zone, tz);
 
if (pos) {
@@ -78,9 +79,9 @@ rcs_set_tz(char *tz, struct rcs_delta *r
tb-tm_hour = 0;
 
if (m != NULL) {
-   tzone = atoi(m);
-   if (tzone = 60)
-   errx(1, %s: not a known time zone, tz);
+   tzone = (int)strtonum(m, 0, 59, errstr);
+   if (errstr)
+   errx(1, %s: not a known minute, m);
 
if ((tb-tm_min + tzone) = 60) {
tb-tm_hour++;



[PATCH]unnecessary typecast in rcs xmalloc

2014-07-30 Thread Fritjof Bornebusch
Hi tech,

there is an unnecessary typecast in xmalloc.c of rcs.

fritjof


Index: xmalloc.c
===
RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v
retrieving revision 1.4
diff -u -p -r1.4 xmalloc.c
--- xmalloc.c   7 Jun 2009 08:39:13 -   1.4
+++ xmalloc.c   20 Jun 2014 10:21:37 -
@@ -32,8 +32,8 @@ xmalloc(size_t size)
ptr = malloc(size);
if (ptr == NULL)
errx(1,
-   xmalloc: out of memory (allocating %lu bytes),
-   (u_long) size);
+   xmalloc: out of memory (allocating %zu bytes),
+   size);
return ptr;
 }
 
@@ -48,8 +48,8 @@ xcalloc(size_t nmemb, size_t size)
errx(1, xcalloc: nmemb * size  SIZE_MAX);
ptr = calloc(nmemb, size);
if (ptr == NULL)
-   errx(1, xcalloc: out of memory (allocating %lu bytes),
-   (u_long)(size * nmemb));
+   errx(1, xcalloc: out of memory (allocating %zu bytes),
+   (size * nmemb));
return ptr;
 }
 
@@ -68,8 +68,8 @@ xrealloc(void *ptr, size_t nmemb, size_t
else
new_ptr = realloc(ptr, new_size);
if (new_ptr == NULL)
-   errx(1, xrealloc: out of memory (new_size %lu bytes),
-   (u_long) new_size);
+   errx(1, xrealloc: out of memory (new_size %zu bytes),
+   new_size);
return new_ptr;
 }
 



Re: [PATCH]unnecessary return in arc4random

2014-05-31 Thread Fritjof Bornebusch
Am I wrong?

On Thu, May 22, 2014 at 04:30:03PM +0200, Fritjof Bornebusch wrote:
 Hi tech,
 
 does this return makes any sense, because it's a void function and the return 
 is at the end of the function.
 
 fritjof
 
 Index: arc4random.c
 ===
 RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v
 retrieving revision 1.30
 diff -u -p -r1.30 arc4random.c
 --- arc4random.c6 May 2014 16:06:33 -   1.30
 +++ arc4random.c22 May 2014 14:21:35 -
 @@ -165,7 +165,6 @@ _rs_random_u32(u_int32_t *val)
 memcpy(val, rs_buf + RSBUFSZ - rs_have, sizeof(*val));
 memset(rs_buf + RSBUFSZ - rs_have, 0, sizeof(*val));
 rs_have -= sizeof(*val);
 -   return;
  }
  
  u_int32_t
 



[PATCH]unnecessary return in arc4random

2014-05-22 Thread Fritjof Bornebusch
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

2014-05-14 Thread Fritjof Bornebusch
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

2014-05-10 Thread Fritjof Bornebusch
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

2014-05-09 Thread Fritjof Bornebusch
On Wed, May 07, 2014 at 08:59:03PM +0200, Fritjof Bornebusch wrote:
 On Wed, May 07, 2014 at 08:05:35PM +0200, J??r??mie Courr??ges-Anglas wrote:
  Fritjof Bornebusch frit...@alokat.org writes:
  
  [...]
  
   Does no one want to check the diff and give me some feedback?
  
  Regardless of the content of your diff, the date of your mail was:
  
Date: Tue, 6 May 2014 22:57:57 +0200
  
  I doubt there are developers that are paid full-time to improve rcs,
  please be more patient.
  
  -- 
  jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
 
 Sorry, you are right.
 Next time I'll be more patient.
 
 fritjof
 
Come on, nobody? 
If my diffs point to the wrong direction, then please tell me!

fritjof



Re: [PATCH] comparison between signed and unsigned in rcs

2014-05-09 Thread Fritjof Bornebusch
On Fri, May 09, 2014 at 12:35:11PM -0400, Kenneth Westerback wrote:
 On 9 May 2014 11:47, Kenneth Westerback kwesterb...@gmail.com wrote:
  On 9 May 2014 11:41, Fritjof Bornebusch frit...@alokat.org wrote:
  On Wed, May 07, 2014 at 08:59:03PM +0200, Fritjof Bornebusch wrote:
  On Wed, May 07, 2014 at 08:05:35PM +0200, J??r??mie Courr??ges-Anglas 
  wrote:
   Fritjof Bornebusch frit...@alokat.org writes:
  
   [...]
  
Does no one want to check the diff and give me some feedback?
  
   Regardless of the content of your diff, the date of your mail was:
  
 Date: Tue, 6 May 2014 22:57:57 +0200
  
   I doubt there are developers that are paid full-time to improve rcs,
   please be more patient.
  
   --
   jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 
   E7EE
 
  Sorry, you are right.
  Next time I'll be more patient.
 
  fritjof
 
  Come on, nobody?
  If my diffs point to the wrong direction, then please tell me!
 
  fritjof
 
 
  No one has the time or interest apparently. Whining about it every
  couple of days does not help motivate people to drop their current
  work to look at your stylistic tweaks.
 
   Ken
 
 I apologize for the unintentionally harsh tone. We are always
 interested in getting diffs.
 
  Ken

No problem.

But if you like OpenRCS as it is and don't want to get any diffs unless 
security fixes, then just say it.
Or if you work on other stuff and you don't have time for that, then just say 
it.

fritjof



Re: [PATCH] comparison between signed and unsigned in rcs

2014-05-09 Thread Fritjof Bornebusch
On Fri, May 09, 2014 at 06:01:52PM +0200, J??r??mie Courr??ges-Anglas wrote:
 Fritjof Bornebusch frit...@alokat.org writes:
 
  Hi tech,
 
 Hi,
 
  if I compile rcs, gcc prints a few warnings like this:
  - comparison between signed and unsigned
  - signed and unsigned type in conditional expression
 
  I'm not quite sure if the typecasts are at the correct place, but these 
  diffs removes the warnings.
 
  fritjof
 
 
  Index: buf.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/buf.c,v
  retrieving revision 1.22
  diff -u -p -r1.22 buf.c
  --- buf.c   6 Jul 2011 15:36:52 -   1.22
  +++ buf.c   6 May 2014 20:56:55 -
  @@ -98,7 +98,7 @@ buf_load(const char *path)
  if (fstat(fd, st) == -1)
  goto out;
   
  -   if (st.st_size  SIZE_MAX) {
  +   if (st.st_size  (off_t)SIZE_MAX) {
  errno = EFBIG;
  goto out;
  }
 
 This would break on 64 bits archs: there, SIZE_MAX casted to an off_t
 is -1 (implementation-defined behavior iirc).
 
 Also it breaks platforms with 32 bits off_t (this does not affect
 OpenBSD).
 
 I think the code is perfectly fine as is, but a possible clarification
 that does not do assumptions about the site of off_t and size_t could
 be:
 
 if ((uintmax_t)st.st_size  (uintmax_t)SIZE_MAX)
   ...
 
  Index: diff.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/diff.c,v
  retrieving revision 1.34
  diff -u -p -r1.34 diff.c
  --- diff.c  16 May 2013 12:44:48 -  1.34
  +++ diff.c  6 May 2014 20:57:07 -
  @@ -432,13 +432,13 @@ prepare(int i, FILE *fd, off_t filesize,
   
  rewind(fd);
   
  -   sz = (filesize = SIZE_MAX ? filesize : SIZE_MAX) / 25;
  +   sz = (filesize = (off_t)SIZE_MAX ? filesize : (off_t)SIZE_MAX) / 
  25;
  if (sz  100)
  sz = 100;
 
 Same problem here.
 
  p = xcalloc(sz + 3, sizeof(*p));
  for (j = 0; (h = readhash(fd, flags));) {
  -   if (j == sz) {
  +   if ((size_t)j == sz) {
  sz = sz * 3 / 2;
  p = xrealloc(p, sz + 3, sizeof(*p));
  }
 
 Declaring j as a size_t would save a cast.
 
  Index: diff3.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
  retrieving revision 1.33
  diff -u -p -r1.33 diff3.c
  --- diff3.c 4 Mar 2012 04:05:15 -   1.33
  +++ diff3.c 6 May 2014 20:57:18 -
  @@ -908,7 +908,7 @@ edscript(int n)
  (void)fseek(fp[2], (long)de[n].new.from, SEEK_SET);
  for (k = de[n].new.to-de[n].new.from; k  0; k-= j) {
  j = k  BUFSIZ ? BUFSIZ : k;
  -   if (fread(block, 1, j, fp[2]) != j)
  +   if ((int)fread(block, 1, j, fp[2]) != j)
  return (-1);
  block[j] = '\0';
  diff_output(%s, block);
 
 Same here.
 
  Index: buf.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/buf.c,v
  retrieving revision 1.22
  diff -u -p -r1.22 buf.c
  --- buf.c   6 Jul 2011 15:36:52 -   1.22
  +++ buf.c   6 May 2014 20:57:30 -
  @@ -98,7 +98,7 @@ buf_load(const char *path)
  if (fstat(fd, st) == -1)
  goto out;
   
  -   if (st.st_size  SIZE_MAX) {
  +   if (st.st_size  (off_t)SIZE_MAX) {
  errno = EFBIG;
  goto out;
  }
 
 
 Duplicated hunk?
 
 -- 
 jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Hi,

thank you for your feedback. I'll add it.

fritjof



Re: [PATCH] rcs merge

2014-05-08 Thread Fritjof Bornebusch
On Thu, May 08, 2014 at 09:45:22AM +0200, Janne Johansson wrote:
 Can't say if this was the motivation here, but some people like to put
 constants before variables for comparisons so as to easily catch the
 difference between
 if (a = 5) ...
 and
 if (5 = a) ..
 when you really meant if (a == 5).
 
You are right, some people like to see constants before variables for 
comparisions.
But if you look later in the code you see something like if (argc != 3) or 
for (; labels  3; labels++).
As you can see the variables are the first parameter for comparisions.

So this diff makes it more consistent what format is used, too.

fritjof 
 
 
 2014-05-08 0:13 GMT+02:00 Fritjof Bornebusch frit...@alokat.org:
 
  Hi tech,
 
  I think labels = 3 is more readable than 3 = labels.
 
  fritjof
 
  Index: merge.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/merge.c,v
  retrieving revision 1.7
  diff -u -p -r1.7 merge.c
  --- merge.c 23 Jul 2010 21:46:05 -  1.7
  +++ merge.c 7 May 2014 22:10:06 -
  @@ -62,7 +62,7 @@ merge_main(int argc, char **argv)
  flags |= MERGE_EFLAG;
  break;
  case 'L':
  -   if (3 = labels)
  +   if (labels = 3)
  errx(D_ERROR, too many -L options);
  label[labels++] = optarg;
  break;
 
 
 
 
 -- 
 May the most significant bit of your life be positive.



[PATCH] rcs: no way to go, after usage was called

2014-05-08 Thread Fritjof Bornebusch
Hi tech,

there is no way you can go, after usage() was called, so dont't do it.

fritjof

Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.216
diff -u -p -r1.216 ci.c
--- ci.c27 Oct 2013 18:31:24 -  1.216
+++ ci.c8 May 2014 19:46:13 -
@@ -97,6 +97,8 @@ checkin_usage(void)
  [-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
  [-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
  [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+   exit(1);
 }
 
 /*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-   exit(1);
}
 
if ((pb.username = getlogin()) == NULL)



Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 -  1.117
+++ co.c8 May 2014 19:57:22 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-   exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-   exit (1);
}
 
if ((username = getlogin()) == NULL)
@@ -229,6 +226,8 @@ checkout_usage(void)
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
  [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
  [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+   exit(1);
 }
 
 /*


Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 -  1.7
+++ merge.c 8 May 2014 20:04:11 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
-   exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc  3) ? not enough : too many);
(usage)();
-   exit(D_ERROR);
}
 
for (; labels  3; labels++)
@@ -118,4 +116,6 @@ merge_usage(void)
 {
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+   exit(D_ERROR);
 }



Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c  28 Jul 2010 09:07:11 -  1.52
+++ rcsclean.c  8 May 2014 20:05:52 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-   exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
(usage)();
-   exit(1);
}
}
 
@@ -104,7 +102,6 @@ rcsclean_main(int argc, char **argv)
if ((dirp = opendir(.)) == NULL) {
warn(opendir);
(usage)();
-   exit(1);
}
 
while ((dp = readdir(dirp)) != NULL) {
@@ -127,6 +124,8 @@ rcsclean_usage(void)
fprintf(stderr,
usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n
[-u[rev]] [-xsuffixes] [-ztz] [file ...]\n);
+
+   exit(1);
 }
 
 static void


Index: rcsdiff.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
retrieving revision 1.79
diff -u -p -r1.79 rcsdiff.c
--- rcsdiff.c   16 Apr 2013 20:24:45 -  1.79
+++ 

Re: [PATCH] comparison between signed and unsigned in rcs

2014-05-07 Thread Fritjof Bornebusch
On Tue, May 06, 2014 at 10:57:57PM +0200, Fritjof Bornebusch wrote:
 Hi tech,
 
 if I compile rcs, gcc prints a few warnings like this:
 - comparison between signed and unsigned
 - signed and unsigned type in conditional expression
 
 I'm not quite sure if the typecasts are at the correct place, but these diffs 
 removes the warnings.
 
 fritjof
 
 
 Index: buf.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/buf.c,v
 retrieving revision 1.22
 diff -u -p -r1.22 buf.c
 --- buf.c   6 Jul 2011 15:36:52 -   1.22
 +++ buf.c   6 May 2014 20:56:55 -
 @@ -98,7 +98,7 @@ buf_load(const char *path)
 if (fstat(fd, st) == -1)
 goto out;
  
 -   if (st.st_size  SIZE_MAX) {
 +   if (st.st_size  (off_t)SIZE_MAX) {
 errno = EFBIG;
 goto out;
 }
 
 Index: diff.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/diff.c,v
 retrieving revision 1.34
 diff -u -p -r1.34 diff.c
 --- diff.c  16 May 2013 12:44:48 -  1.34
 +++ diff.c  6 May 2014 20:57:07 -
 @@ -432,13 +432,13 @@ prepare(int i, FILE *fd, off_t filesize,
  
 rewind(fd);
  
 -   sz = (filesize = SIZE_MAX ? filesize : SIZE_MAX) / 25;
 +   sz = (filesize = (off_t)SIZE_MAX ? filesize : (off_t)SIZE_MAX) / 25;
 if (sz  100)
 sz = 100;
  
 p = xcalloc(sz + 3, sizeof(*p));
 for (j = 0; (h = readhash(fd, flags));) {
 -   if (j == sz) {
 +   if ((size_t)j == sz) {
 sz = sz * 3 / 2;
 p = xrealloc(p, sz + 3, sizeof(*p));
 }
 
 Index: diff3.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
 retrieving revision 1.33
 diff -u -p -r1.33 diff3.c
 --- diff3.c 4 Mar 2012 04:05:15 -   1.33
 +++ diff3.c 6 May 2014 20:57:18 -
 @@ -908,7 +908,7 @@ edscript(int n)
 (void)fseek(fp[2], (long)de[n].new.from, SEEK_SET);
 for (k = de[n].new.to-de[n].new.from; k  0; k-= j) {
 j = k  BUFSIZ ? BUFSIZ : k;
 -   if (fread(block, 1, j, fp[2]) != j)
 +   if ((int)fread(block, 1, j, fp[2]) != j)
 return (-1);
 block[j] = '\0';
 diff_output(%s, block);
 
 Index: buf.c
 ===
 RCS file: /cvs/src/usr.bin/rcs/buf.c,v
 retrieving revision 1.22
 diff -u -p -r1.22 buf.c
 --- buf.c   6 Jul 2011 15:36:52 -   1.22
 +++ buf.c   6 May 2014 20:57:30 -
 @@ -98,7 +98,7 @@ buf_load(const char *path)
 if (fstat(fd, st) == -1)
 goto out;
  
 -   if (st.st_size  SIZE_MAX) {
 +   if (st.st_size  (off_t)SIZE_MAX) {
 errno = EFBIG;
 goto out;
 }
 

Does no one want to check the diff and give me some feedback?

fritjof



Re: [PATCH] comparison between signed and unsigned in rcs

2014-05-07 Thread Fritjof Bornebusch
On Wed, May 07, 2014 at 08:05:35PM +0200, J??r??mie Courr??ges-Anglas wrote:
 Fritjof Bornebusch frit...@alokat.org writes:
 
 [...]
 
  Does no one want to check the diff and give me some feedback?
 
 Regardless of the content of your diff, the date of your mail was:
 
   Date: Tue, 6 May 2014 22:57:57 +0200
 
 I doubt there are developers that are paid full-time to improve rcs,
 please be more patient.
 
 -- 
 jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Sorry, you are right.
Next time I'll be more patient.

fritjof



[PATCH] rcs void casts

2014-05-07 Thread Fritjof Bornebusch
Hi tech,

there are a few void casts in rcs. But I have a question about that.
Are these casts really necessary? I've read that the compiler warns, because of 
unused variables.
But no compiler warnings about that on amd64.

That's why I just added this small diff, in order to get feedback if the casts
are necessary or not.

fritjof

Index: rlog.c
===
RCS file: /cvs/src/usr.bin/rcs/rlog.c,v
retrieving revision 1.67
diff -u -p -r1.67 rlog.c
--- rlog.c  7 Jan 2014 14:08:16 -   1.67
+++ rlog.c  7 May 2014 20:24:37 -
@@ -517,7 +517,7 @@ rlog_rev_print(struct rcs_delta *rdp)
fmt = %Y/%m/%d %H:%M:%S;
}
 
-   (void)strftime(timeb, sizeof(timeb), fmt, t);
+   strftime(timeb, sizeof(timeb), fmt, t);
 
printf(\ndate: %s;  author: %s;  state: %s;, timeb, rdp-rd_author,
rdp-rd_state);
@@ -556,7 +556,7 @@ rlog_rev_print(struct rcs_delta *rdp)
TAILQ_FOREACH(rb, (rdp-rd_branches), rb_list) {
RCSNUM *branch;
branch = rcsnum_revtobr(rb-rb_num);
-   (void)rcsnum_tostr(branch, numb, sizeof(numb));
+   rcsnum_tostr(branch, numb, sizeof(numb));
printf(  %s;, numb);
rcsnum_free(branch);
}



Re: [PATCH] rcs void casts

2014-05-07 Thread Fritjof Bornebusch
On Wed, May 07, 2014 at 10:58:03PM +0200, Ingo Schwarze wrote:
 Hi Fritjof,
 
 Fritjof Bornebusch wrote on Wed, May 07, 2014 at 10:32:05PM +0200:
 
  there are a few void casts in rcs. But I have a question about that.
  Are these casts really necessary?
 
 No, they are not necessary.
 
  I've read that the compiler warns, because of unused variables.
  But no compiler warnings about that on amd64.
 
 Usually, you don't choose your style to please static analysis tools -
 there are too many of them, of too varying quality and signal-to-noise
 ratio, to please them all.
 
 Rather, it makes sense to tune static analysis tools, including
 compilers, to warn about dangerous style, but to not warn about style
 that is merely a matter of taste.
 
 That said, for functions that usually require checking of return
 values, the void cast can be used to indicate this call was audited,
 and ignoring the return value is considered safe, either because
 this specific call cannot actually fail, or because failure does
 not need explicit handling at this point.  Typical functions where
 this makes sense are snprintf(3), strlcpy(3), strlcat(3).  So i
 clearly wouldn't remove the cast from strftime() unless you have
 reason to assume that no proper audit was done, or this is broken
 and needs fixing.  In this case, the buffer is large enough, so
 the cast should stay.
 
 On the other hand, on functions that hardly ever require checking
 the return code, casting to void is pointless, so i tend to
 remove such casts when editing nearby code.  A typical example
 is (void)close(fd); which is rarely more helpful than
 just close(fd);.
 
 In this case, the rcsnum_tostr() return value can't be used for error
 checking in the first place, that function calls errx(3) on failure,
 so when editing nearby code, i'd probably remove that cast.  But it
 doesn't warrant a commit on it's own imho.
 
 Yours,
   Ingo
 

I see, thank you for the explanations.
It was just for me to make it clear when (void) is used.

fritjof

 
  Index: rlog.c
  ===
  RCS file: /cvs/src/usr.bin/rcs/rlog.c,v
  retrieving revision 1.67
  diff -u -p -r1.67 rlog.c
  --- rlog.c  7 Jan 2014 14:08:16 -   1.67
  +++ rlog.c  7 May 2014 20:24:37 -
  @@ -517,7 +517,7 @@ rlog_rev_print(struct rcs_delta *rdp)
  fmt = %Y/%m/%d %H:%M:%S;
  }
   
  -   (void)strftime(timeb, sizeof(timeb), fmt, t);
  +   strftime(timeb, sizeof(timeb), fmt, t);
   
  printf(\ndate: %s;  author: %s;  state: %s;, timeb, 
  rdp-rd_author,
  rdp-rd_state);
  @@ -556,7 +556,7 @@ rlog_rev_print(struct rcs_delta *rdp)
  TAILQ_FOREACH(rb, (rdp-rd_branches), rb_list) {
  RCSNUM *branch;
  branch = rcsnum_revtobr(rb-rb_num);
  -   (void)rcsnum_tostr(branch, numb, sizeof(numb));
  +   rcsnum_tostr(branch, numb, sizeof(numb));
  printf(  %s;, numb);
  rcsnum_free(branch);
  }



[PATCH] rcs merge

2014-05-07 Thread 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;



[PATCH] rcs stored values never read

2014-05-06 Thread Fritjof Bornebusch
Hi tech,

there are some never read values in rcs.


fritjof 

Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 -  1.117
+++ co.c6 May 2014 19:57:15 -
@@ -56,7 +56,6 @@ checkout_main(int argc, char **argv)
 
flags = ret = 0;
kflag = RCS_KWEXP_ERR;
-   rev = RCS_HEAD_REV;
rev_str = NULL;
author = date = state = NULL;
 
@@ -257,7 +256,7 @@ checkout_rev(RCSFILE *file, RCSNUM *frev
time_t rcsdate, givendate;
RCSNUM *rev;
 
-   rcsdate = givendate = -1;
+   givendate = -1;
if (date != NULL  (givendate = date_parse(date)) == -1) {
warnx(invalid date: %s, date);
return -1;


Index: diff.c
===
RCS file: /cvs/src/usr.bin/rcs/diff.c,v
retrieving revision 1.34
diff -u -p -r1.34 diff.c
--- diff.c  16 May 2013 12:44:48 -  1.34
+++ diff.c  6 May 2014 20:00:43 -
@@ -1302,7 +1302,7 @@ dump_unified_vec(FILE *f1, FILE *f2, int
if (context_vec_start  context_vec_ptr)
return;
 
-   b = d = 0;  /* gcc */
+   d = 0;  /* gcc */
lowa = MAX(1, cvp-a - diff_context);
upb = MIN(len[0], context_vec_ptr-b + diff_context);
lowc = MAX(1, cvp-c - diff_context);

Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 -  1.7
+++ merge.c 6 May 2014 20:03:52 -
@@ -40,7 +40,6 @@ merge_main(int argc, char **argv)
BUF *bp;
 
flags = labels = 0;
-   status = D_ERROR;
 
/*
 * Using getopt(3) and not rcs_getopt() because merge(1)


Index: rcs.c
===
RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
retrieving revision 1.80
diff -u -p -r1.80 rcs.c
--- rcs.c   7 Jan 2014 14:08:16 -   1.80
+++ rcs.c   6 May 2014 20:06:00 -
@@ -214,7 +214,6 @@ rcs_write(RCSFILE *rfp)
int fd;
 
fn = NULL;
-   fd = -1;
 
if (rfp-rf_flags  RCS_SYNCED)
return;


Index: rcsmerge.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsmerge.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsmerge.c
--- rcsmerge.c  23 Jul 2010 21:46:05 -  1.52
+++ rcsmerge.c  6 May 2014 20:08:02 -
@@ -44,7 +44,6 @@ rcsmerge_main(int argc, char **argv)
BUF *bp;
 
flags = 0;
-   kflag = RCS_KWEXP_ERR;
status = D_ERROR;
rev1 = rev2 = NULL;
rev_str1 = rev_str2 = NULL;


Index: rcsparse.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsparse.c,v
retrieving revision 1.9
diff -u -p -r1.9 rcsparse.c
--- rcsparse.c  3 Jun 2013 17:04:35 -   1.9
+++ rcsparse.c  6 May 2014 20:10:23 -
@@ -915,7 +915,6 @@ rcsparse_token(RCSFILE *rfp, int allowed
} while (isspace(c));
 
pdp-rp_msglineno = pdp-rp_lineno;
-   type = 0;
switch (c) {
case '@':
ret = rcsparse_string(rfp, allowed);
@@ -1104,7 +1103,6 @@ rcsparse(RCSFILE *rfp, struct rcs_sectio
int i, token;
 
pdp = (struct rcs_pdata *)rfp-rf_pdata;
-   i = 0;
 
token = 0;
for (i = 0; sec[i].token != 0; i++) {


Index: rcsutil.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsutil.c,v
retrieving revision 1.39
diff -u -p -r1.39 rcsutil.c
--- rcsutil.c   16 Apr 2013 20:24:45 -  1.39
+++ rcsutil.c   6 May 2014 20:12:13 -
@@ -157,8 +157,6 @@ rcs_choosefile(const char *filename, cha
char *p, *ext, name[MAXPATHLEN], *next, *ptr, rcsdir[MAXPATHLEN],
*suffixes, rcspath[MAXPATHLEN];
 
-   fd = -1;
-
/*
 * If `filename' contains a directory, `rcspath' contains that
 * directory, including a trailing slash.  Otherwise `rcspath'


Index: rlog.c
===
RCS file: /cvs/src/usr.bin/rcs/rlog.c,v
retrieving revision 1.67
diff -u -p -r1.67 rlog.c
--- rlog.c  7 Jan 2014 14:08:16 -   1.67
+++ rlog.c  6 May 2014 20:14:50 -
@@ -433,7 +433,7 @@ rlog_rev_print(struct rcs_delta *rdp)
struct rcs_branch *rb;
struct rcs_delta *nrdp;
 
-   i = found = 0;
+   found = 0;
author = NULL;
 
/* -l[lockers] */



[PATCH] comparison between signed and unsigned in rcs

2014-05-06 Thread Fritjof Bornebusch
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;
}



[PATCH] mail assignment never read

2014-04-24 Thread Fritjof Bornebusch
Hi tech,

this assignment is never read.

Fritjof

Index: collect.c
===
RCS file: /cvs/src/usr.bin/mail/collect.c,v
retrieving revision 1.34
diff -u -p -r1.34 collect.c
--- collect.c   17 Jan 2014 18:42:30 -  1.34
+++ collect.c   24 Apr 2014 21:31:38 -
@@ -65,7 +65,6 @@ collect(struct header *hp, int printhead
collf = NULL;
eofcount = 0;
hadintr = 0;
-   lastlong = 0;
longline = 0;
if ((cp = value(escape)) != NULL)
escape = *cp;



[patch] cvs some values never read

2014-04-23 Thread Fritjof Bornebusch
Hi tech,

there are some set operations, which are never read.

Fritjof

Index: rcsparse.c
===
RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v
retrieving revision 1.7
diff -u -p -r1.7 rcsparse.c
--- rcsparse.c  3 Jun 2013 17:04:35 -   1.7
+++ rcsparse.c  23 Apr 2014 17:09:44 -
@@ -916,7 +916,6 @@ rcsparse_token(RCSFILE *rfp, int allowed
} while (isspace(c));
 
pdp-rp_msglineno = pdp-rp_lineno;
-   type = 0;
switch (c) {
case '@':
ret = rcsparse_string(rfp, allowed);
@@ -1105,7 +1104,6 @@ rcsparse(RCSFILE *rfp, struct rcs_sectio
int i, token;
 
pdp = (struct rcs_pdata *)rfp-rf_pdata;
-   i = 0;
 
token = 0;
for (i = 0; sec[i].token != 0; i++) {



Index: diff_internals.c
===
RCS file: /cvs/src/usr.bin/cvs/diff_internals.c,v
retrieving revision 1.34
diff -u -p -r1.34 diff_internals.c
--- diff_internals.c1 Apr 2011 17:25:26 -   1.34
+++ diff_internals.c23 Apr 2014 17:10:19 -
@@ -1375,7 +1375,7 @@ dump_unified_vec(FILE *f1, FILE *f2, int
if (context_vec_start  context_vec_ptr)
return;
 
-   b = d = 0;  /* gcc */
+   d = 0;  /* gcc */
lowa = MAX(1, cvp-a - diff_context);
upb = MIN(len[0], context_vec_ptr-b + diff_context);
lowc = MAX(1, cvp-c - diff_context);


Index: getlog.c
===
RCS file: /cvs/src/usr.bin/cvs/getlog.c,v
retrieving revision 1.97
diff -u -p -r1.97 getlog.c
--- getlog.c8 Jan 2014 13:23:55 -   1.97
+++ getlog.c23 Apr 2014 17:11:59 -
@@ -318,7 +318,7 @@ log_rev_print(struct rcs_delta *rdp)
struct rcs_branch *rb;
struct rcs_delta *nrdp;
 
-   i = found = 0;
+   found = 0;
 
/* -s states */
if (runflags  L_STATES) {


Index: rcs.c
===
RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
retrieving revision 1.311
diff -u -p -r1.311 rcs.c
--- rcs.c   8 Jan 2014 13:23:55 -   1.311
+++ rcs.c   23 Apr 2014 17:21:39 -
@@ -293,8 +293,6 @@ rcs_write(RCSFILE *rfp)
size_t len;
int fd, saved_errno;
 
-   fd = -1;
-
if (rfp-rf_flags  RCS_SYNCED)
return;
 
@@ -2229,7 +2227,6 @@ rcs_kwexp_line(char *rcsfile, struct rcs
return;
 
c = line-l_line;
-   found = 0;
/* Final character in buffer. */
fin = c + len - 1;
 
@@ -2503,7 +2500,6 @@ rcs_kwexp_line(char *rcsfile, struct rcs
lp-l_len = strlen(lp-l_line);
TAILQ_INSERT_AFTER((lines-l_lines), cur, lp,
l_list);
-   cur = lp;
 
end = line-l_line + line-l_len - 1;



Re: [patch] cvs some values never read

2014-04-23 Thread Fritjof Bornebusch

 * Fritjof Bornebusch frit...@alokat.org [2014-04-23 19:30]:
  there are some set operations, which are never read.
 
  RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v
 
 guess we need to decide what to do with opencvs really. either there
 is someone who cares and picks it up, or we can straight delete it. it
 hasn't moved forward in years, and I have a hard time seeing it going
 anywhere (except Attic). But that's just me, of course.
 
If opencvs is going to be deleted, what is the alternative? gnucvs?
 --
 Henning Brauer, h...@bsws.de, henn...@openbsd.org
 BS Web Services GmbH, http://bsws.de, Full-Service ISP
 Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to 
Fully Managed
 Henning Brauer Consulting, http://henningbrauer.com/
 
Fritjof



Re: [patch] cvs some values never read

2014-04-23 Thread Fritjof Bornebusch

 * Fritjof Bornebusch frit...@alokat.org [2014-04-23 20:15]:
  
* Fritjof Bornebusch frit...@alokat.org [2014-04-23 19:30]:
 there are some set operations, which are never read.

 RCS file: /cvs/src/usr.bin/cvs/rcsparse.c,v

guess we need to decide what to do with opencvs really. either 
there
is someone who cares and picks it up, or we can straight 
delete it. it
hasn't moved forward in years, and I have a hard time seeing 
it going
anywhere (except Attic). But that's just me, of course.

  If opencvs is going to be deleted, what is the alternative? gnucvs?
 
 err, that's what we've been using all the time. It has never become
 ready.
 
 revision 1.114
 date: 2010/06/26 03:59:34;  author: deraadt;  state: Exp;  lines: +2 
-2;
 disable opencvs; maintainers went bye bye
 
Ah, I see.
 -- 
 Henning Brauer, h...@bsws.de, henn...@openbsd.org
 BS Web Services GmbH, http://bsws.de, Full-Service ISP
 Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to 
Fully Managed
 Henning Brauer Consulting, http://henningbrauer.com/
 



[PATCH] ssh: variables never read

2014-04-23 Thread Fritjof Bornebusch
Hi tech,

there are some unread set operations in the ssh code.

Fritjof

Index: clientloop.c
===
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.258
diff -u -p -r1.258 clientloop.c
--- clientloop.c2 Feb 2014 03:44:31 -   1.258
+++ clientloop.c23 Apr 2014 19:50:18 -
@@ -934,7 +934,6 @@ process_cmdline(void)
 
/* XXX update list of forwards in options */
if (delete) {
-   cancel_port = 0;
cancel_host = hpdelim(s);  /* may be NULL */
if (s != NULL) {
cancel_port = a2port(s);


Index: krl.c
===
RCS file: /cvs/src/usr.bin/ssh/krl.c,v
retrieving revision 1.14
diff -u -p -r1.14 krl.c
--- krl.c   31 Jan 2014 16:39:19 -  1.14
+++ krl.c   23 Apr 2014 19:55:50 -
@@ -496,7 +496,6 @@ choose_next_state(int current_state, u_i
if (cost_bitmap_restart  cost) {
new_state = KRL_SECTION_CERT_SERIAL_BITMAP;
*force_new_section = 1;
-   cost = cost_bitmap_restart;
}
debug3(%s: contig %llu last_gap %llu next_gap %llu final %d, 
costs:
list %llu range %llu bitmap %llu new bitmap %llu, 
@@ -957,7 +956,6 @@ ssh_krl_from_blob(Buffer *buf, struct ss
/* Not interested for now. */
continue;
}
-   sig_seen = 1;
/* First string component is the signing key */
if ((key = key_from_blob(blob, blen)) == NULL) {
error(%s: invalid signature key, __func__);


Index: readconf.c
===
RCS file: /cvs/src/usr.bin/ssh/readconf.c,v
retrieving revision 1.219
diff -u -p -r1.219 readconf.c
--- readconf.c  23 Apr 2014 12:42:34 -  1.219
+++ readconf.c  23 Apr 2014 20:01:26 -
@@ -1255,7 +1255,7 @@ parse_int:
if (!arg || *arg == '\0')
fatal(%.200s line %d: Missing ControlPersist
 argument., filename, linenum);
-   value = 0;
+
value2 = 0; /* timeout */
if (strcmp(arg, no) == 0 || strcmp(arg, false) == 0)
value = 0;


Index: scp.c
===
RCS file: /cvs/src/usr.bin/ssh/scp.c,v
retrieving revision 1.179
diff -u -p -r1.179 scp.c
--- scp.c   20 Nov 2013 20:53:10 -  1.179
+++ scp.c   23 Apr 2014 20:05:22 -
@@ -818,7 +818,6 @@ next:   if (fd != -1) {
if (fd != -1) {
if (close(fd)  0  !haderr)
haderr = errno;
-   fd = -1;
}
if (!haderr)
(void) atomicio(vwrite, remout, , 1);


Index: sftp.c
===
RCS file: /cvs/src/usr.bin/ssh/sftp.c,v
retrieving revision 1.160
diff -u -p -r1.160 sftp.c
--- sftp.c  22 Apr 2014 10:07:12 -  1.160
+++ sftp.c  23 Apr 2014 20:11:54 -
@@ -1222,7 +1222,6 @@ parse_args(const char **cpp, int *ignore
*aflag = *fflag = *hflag = *iflag = *lflag = *pflag = 0;
*rflag = *sflag = 0;
*path1 = *path2 = NULL;
-   optidx = 1;
switch (cmdnum) {
case I_GET:
case I_REGET:



  1   2   >