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

[Jason Smethers <jason@xxxxxxxxxxxx>]

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