Re: [PATCH] libexec/fingerd WARNS=6 cleanup

[Joe Talbott <josepht@xxxxxxxxxx>]

On Sat, Apr 30, 2005 at 12:50:51PM +0100, Liam J. Foy wrote:
> Hello Joe!
> 
> > +			if (secure && ap == &av[3])
> > +				err(1, "must provide username");
> >  			break;
> 
> err() will print the error message with errno. Of course, errno
> is not set here. This will lead to an invalid err message. You
> really want to use errx() which will not use errno.
> 
> > +		if (secure && strchr(*ap, '@')) 
> > +			err(1, "forwarding service denied");
> >  

Fixed.

Joe

Index: Makefile
===================================================================
RCS file: /home/dcvs/src/libexec/fingerd/Makefile,v
retrieving revision 1.2
diff -u -r1.2 Makefile
--- Makefile	17 Jun 2003 04:27:07 -0000	1.2
+++ Makefile	27 Apr 2005 13:03:37 -0000
@@ -6,5 +6,6 @@
 DPADD=	${LIBUTIL}
 LDADD=	-lutil
 MAN=	fingerd.8
+WARNS?=	6
 
 .include <bsd.prog.mk>
Index: fingerd.c
===================================================================
RCS file: /home/dcvs/src/libexec/fingerd/fingerd.c,v
retrieving revision 1.3
diff -u -r1.3 fingerd.c
--- fingerd.c	14 Nov 2003 03:54:29 -0000	1.3
+++ fingerd.c	30 Apr 2005 17:03:18 -0000
@@ -42,31 +42,38 @@
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
-#include <errno.h>
 
-#include <unistd.h>
-#include <syslog.h>
+#include <err.h>
+#include <errno.h>
 #include <libutil.h>
 #include <netdb.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <strings.h>
+#include <sysexits.h>
+#include <syslog.h>
+#include <unistd.h>
 #include "pathnames.h"
 
 void logerr (const char *, ...);
 
 int
-main(argc, argv)
-	int argc;
-	char *argv[];
+main(int argc, char *argv[])
 {
-	register FILE *fp;
-	register int ch;
-	register char *lp;
+	FILE *fp;
+	int ch;
+	char *lp;
 	struct sockaddr_storage ss;
-	int p[2], logging, secure, sval;
+	int p[2];
+	int logging;
+	int secure;
+	int sval;
 #define	ENTRIES	50
-	char **ap, *av[ENTRIES + 1], **comp, line[1024], *prog;
+	const char **ap;
+	const char *av[ENTRIES + 1];
+	const char ** volatile comp;
+	char line[1024];
+	const char * volatile prog;
 	char rhost[MAXHOSTNAMELEN];
 
 	prog = _PATH_FINGER;
@@ -85,6 +92,7 @@
 			secure = 1;
 			break;
 		case '?':
+			/* FALLTHROUGH */
 		default:
 			logerr("illegal option -- %c", optopt);
 		}
@@ -95,13 +103,13 @@
 	{
 		int one = 1;
 		if (setsockopt(STDOUT_FILENO, IPPROTO_TCP, TCP_NOPUSH, &one, 
-			       sizeof one) < 0) {
+			       sizeof(one)) < 0) {
 			logerr("setsockopt(TCP_NOPUSH) failed: %m");
 		}
 	}
 
 	if (!fgets(line, sizeof(line), stdin))
-		exit(1);
+		exit(EX_IOERR);
 
 	if (logging) {
 		char *t;
@@ -134,16 +142,12 @@
 	for (lp = line, ap = &av[3];;) {
 		*ap = strtok(lp, " \t\r\n");
 		if (!*ap) {
-			if (secure && ap == &av[3]) {
-				puts("must provide username\r\n");
-				exit(1);
-			}
+			if (secure && ap == &av[3])
+				errx(1, "must provide username");
 			break;
 		}
-		if (secure && strchr(*ap, '@')) {
-			puts("forwarding service denied\r\n");
-			exit(1);
-		}
+		if (secure && strchr(*ap, '@')) 
+			errx(1, "forwarding service denied");
 
 		/* RFC742: "/[Ww]" == "-l" */
 		if ((*ap)[0] == '/' && ((*ap)[1] == 'W' || (*ap)[1] == 'w')) {
@@ -157,7 +161,7 @@
 		lp = NULL;
 	}
 
-	if (lp = strrchr(prog, '/'))
+	if ((lp = strrchr(prog, '/')) != NULL)
 		*comp = ++lp;
 	else
 		*comp = prog;
@@ -166,18 +170,18 @@
 
 	switch(vfork()) {
 	case 0:
-		(void)close(p[0]);
+		close(p[0]);
 		if (p[1] != 1) {
-			(void)dup2(p[1], 1);
-			(void)close(p[1]);
+			dup2(p[1], 1);
+			close(p[1]);
 		}
-		execv(prog, comp);
+		execv(prog, __DECONST(char **, comp));
 		logerr("execv: %s: %s", prog, strerror(errno));
-		_exit(1);
+		_exit(EX_UNAVAILABLE);
 	case -1:
 		logerr("fork: %s", strerror(errno));
 	}
-	(void)close(p[1]);
+	close(p[1]);
 	if (!(fp = fdopen(p[0], "r")))
 		logerr("fdopen: %s", strerror(errno));
 	while ((ch = getc(fp)) != EOF) {
@@ -185,32 +189,18 @@
 			putchar('\r');
 		putchar(ch);
 	}
-	exit(0);
+	exit(EX_OK);
 }
 
-#if __STDC__
 #include <stdarg.h>
-#else
-#include <varargs.h>
-#endif
 
 void
-#if __STDC__
 logerr(const char *fmt, ...)
-#else
-logerr(fmt, va_alist)
-	char *fmt;
-        va_dcl
-#endif
 {
 	va_list ap;
-#if __STDC__
 	va_start(ap, fmt);
-#else
-	va_start(ap);
-#endif
-	(void)vsyslog(LOG_ERR, fmt, ap);
+	vsyslog(LOG_ERR, fmt, ap);
 	va_end(ap);
-	exit(1);
+	exit(EX_UNAVAILABLE);
 	/* NOTREACHED */
 }