Re: cvs commit: src/usr.bin/rev

["Liam J. Foy" <liamfoy@xxxxxxxxxxxxx>]

On Tue, 2004-12-14 at 15:42 -0600, Jason Smethers wrote:
> Liam J. Foy wrote:
> > Hey guys. Regarding Joerg/Matt suggestion, I have re-wrote most of rev.
> > I have changed a few minor things however. Joerg has seen this and is ok
> > with it. If anyone can see any problems, please write back. If no
> > problems are noticed I will most likely commit it later tonight.
> > 
> > Patch:
> > 
> > Index: Makefile
> > ===================================================================
> > RCS file: /home/dcvs/src/usr.bin/rev/Makefile,v
> > retrieving revision 1.1
> > diff -u -r1.1 Makefile
> > --- Makefile	17 Jun 2003 02:56:25 -0000	1.1
> > +++ Makefile	13 Dec 2004 22:25:53 -0000
> > @@ -2,4 +2,5 @@
> >  
> >  PROG=	rev
> >  
> > +WARNS?=	6
> >  .include <bsd.prog.mk>
> > Index: rev.c
> > ===================================================================
> > RCS file: /home/dcvs/src/usr.bin/rev/rev.c,v
> > retrieving revision 1.5
> > diff -u -r1.5 rev.c
> > --- rev.c	13 Dec 2004 17:43:57 -0000	1.5
> > +++ rev.c	14 Dec 2004 17:16:07 -0000
> > @@ -38,21 +38,20 @@
> >  #include <sys/types.h>
> >  
> >  #include <err.h>
> > -#include <errno.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> > -#include <string.h>
> >  #include <unistd.h>
> >  
> >  static void	usage(void);
> > +static int	dorev(FILE *, const char *);
> >  
> >  int
> >  main(int argc, char **argv)
> >  {
> > -	const char *p, *t;
> >  	FILE *fp;
> > -	size_t len;
> > -	int ch, rval;
> > +	int ch, i, rval;
> > +
> > +	rval = 0;
> >  
> >  	while ((ch = getopt(argc, argv, "")) != -1)
> >  		switch(ch) {
> > @@ -64,36 +63,20 @@
> >  	argc -= optind;
> >  	argv += optind;
> >  
> > -	fp = stdin;
> > -	rval = 0;
> > -	do {
> > -		if (*argv) {
> > -			if ((fp = fopen(*argv, "r")) == NULL) {
> > -				warn("%s", *argv);
> > -				rval = 1;
> > -				++argv;
> > -				continue;
> > -			}
> > -		}
> > +	if (argc == 0)
> > +		rval += dorev(stdin, "stdin");
> >  
> > -		while ((p = fgetln(fp, &len)) != NULL) {
> > -			if (p[len - 1] == '\n')
> > -				--len;
> > -			t = p + len - 1;
> > -			for (t = p + len - 1; t >= p; --t)
> > -				putchar(*t);
> > -			putchar('\n');
> > -		}
> > -		if (ferror(fp)) {
> > -			warn("%s", *argv);
> > -			/* Reset error indicator */
> > -			clearerr(fp);
> > -			rval = 1;
> > +	for (i = 0; i < argc; ++i) {
> > +		if ((fp = fopen(argv[i], "r")) != NULL) {
> > +			rval += dorev(fp, argv[i]);
> > +			fclose(fp);
> > +		} else {
> > +			warn("failed to open %s", argv[i]);
> > +			rval++ ;
> >  		}
> > -		++argv;
> > -		fclose(fp);
> > -	} while (*argv);
> > -	exit(rval);
> > +	}
> > +
> > +	exit(rval != 0 ? 1 : 0);
> >  }
> 
> Why not simply use a return statement to exit main()? How antiquated a 
> system do we expect the code in the project to run on? What systems out 
> there don't support the functionality of exiting the program with a 
> return statement instead of a call to exit()?
> 
> Second, OR'ing instead of adding the value returned by dorev() will 
> achieve the functionality you want without doing any tests.
> 
> - Jason

I actually prefere my way due to ease of reading and debugging. Doing it
your way will offer no major benefit anyway except making the code a
little harder to read/understand.

Cheers,
-- 
Liam J. Foy
<liamfoy@xxxxxxxxxxxxx>
Say it in the living years...