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

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

On Mon, 2004-12-13 at 13:28 -0800, Matthew Dillon wrote:
> :Hehe. Sorry for the late reply everyone. I have been busy with bits
> :and bobs. Sorry everyone about the mistake I made. I think
> :- Hide quoted text -
> :
> :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       13 Dec 2004 18:53:33 -0000
> :@@ -90,7 +90,9 @@
> :                       clearerr(fp);
> :                       rval = 1;
> :               }
> :-               ++argv;
> :+               if (argv != NULL)
> :+                       ++argv;
> :+
> :               fclose(fp);
> :       } while (*argv);
> :       exit(rval);
> :
> :will fix it correct? thanks to tomokazu for noticing it.
> :
> :Cheers,
> :
> :-- 
> :Liam J. Foy
> 
>     Why don't you get rid of the do {...} while entirely and just make it
>     a standard for() loop, with a 0-argument check for stdin and make
>     the fgetln() loop its own procedure that you simply pass a FILE *
>     into.  The original code was very badly written.
> 
>     e.g.
> 
>     for (i = 1; i < argc; ++i) {
> 	if ((fp = fopen ...) != NULL ) {
> 	    dofile(fp);
> 	    fclose(fp);
> 	} else {
> 	    warn ...
> 	}
>     }
>     if (argc == 1)
> 	dofile(stdin);
> 
>     dofile(FILE *fp) 
>     {
> 	...
>     }
> 
> 					-Matt
> 					Matthew Dillon 
> 					<dillon@xxxxxxxxxxxxx>

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);
 }
 
 static void
@@ -102,3 +85,28 @@
 	fprintf(stderr, "usage: rev [file ...]\n");
 	exit(1);
 }
+
+static int 
+dorev(FILE *fp, const char *filepath)
+{
+	const char *p, *t;
+	size_t len;
+	
+	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", filepath);
+		/* Reset error indicator */
+		clearerr(fp);
+		return (1);
+	}
+
+	return (0);
+}

Cheers,

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