[PATCH] sbin/ip6fw WARNS=6

[Sepherosa Ziehau <sepherosa@xxxxxxxxx>]

WARNS=6 cleanup

1) remove unused global variable (lineno)
2) add `const'
3) staticize functions
4) adjust {} arround if/else/switch
5) make some index variable unsigned(size_t)
6) add () after sizeof to make code clear
7) rename stack varible to avoid shadowing
8) merge system call return value check into `if' clause, which eliminates some stack variable
9) warn("%s", "XXXX") --> warn("XXXX")
10) in main()
    add q_opt to avoid excessive strdup() call in argument parse
    assign argv[optind - 2] to non-const variable to bypass the const warning


Index: ip6fw.c
===================================================================
RCS file: /opt/df_cvs/src/sbin/ip6fw/ip6fw.c,v
retrieving revision 1.8
diff -u -r1.8 ip6fw.c
--- ip6fw.c	18 Dec 2004 21:43:38 -0000	1.8
+++ ip6fw.c	23 Apr 2005 08:08:49 -0000
@@ -83,8 +83,6 @@
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
 
-int 		lineno = -1;
-
 int 		s;				/* main RAW socket 	   */
 int 		do_resolv=0;			/* Would try to resolv all */
 int		do_acct=0;			/* Show packet/byte count  */
@@ -93,8 +91,8 @@
 int		do_force=0;			/* Don't ask for confirmation */
 
 struct icmpcode {
-	int	code;
-	char	*str;
+	int		 code;
+	const char	*str;
 };
 
 static struct icmpcode icmp6codes[] = {
@@ -138,7 +136,8 @@
 
 static int pl2m[9] = { 0x00, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff };
 
-struct in6_addr *plen2mask(int n)
+static struct in6_addr *
+plen2mask(int n)
 {
 	static struct in6_addr ia;
 	u_char	*p;
@@ -157,7 +156,7 @@
 	return &ia;
 }
 
-void
+static void
 print_port(u_char prot, u_short port, const char *comma)
 {
 	struct servent *se;
@@ -183,7 +182,7 @@
 }
 
 static void
-print_iface(char *key, union ip6_fw_if *un, int byname)
+print_iface(const char *key, union ip6_fw_if *un, int byname)
 {
 
 	if (byname) {
@@ -210,7 +209,7 @@
 static void
 show_ip6fw(struct ip6_fw *chain)
 {
-	char *comma;
+	const char *comma;
 	struct hostent *he;
 	struct protoent *pe;
 	int i, mb;
@@ -225,22 +224,18 @@
 	if (do_acct)
 		printf("%10lu %10lu ",chain->fw_pcnt,chain->fw_bcnt);
 
-	if (do_time)
-	{
-		if (chain->timestamp)
-		{
+	if (do_time) {
+		if (chain->timestamp) {
 			char timestr[30];
 
 			strcpy(timestr, ctime((time_t *)&chain->timestamp));
 			*strchr(timestr, '\n') = '\0';
 			printf("%s ", timestr);
-		}
-		else
+		} else
 			printf("                         ");
 	}
 
-	switch (chain->fw_flg & IPV6_FW_F_COMMAND)
-	{
+	switch (chain->fw_flg & IPV6_FW_F_COMMAND) {
 		case IPV6_FW_F_ACCEPT:
 			printf("allow");
 			break;
@@ -415,7 +410,7 @@
 		if (chain->fw_tcpnf & IPV6_FW_TCPF_URG)  PRINTFLG("!urg");
 	}
 	if (chain->fw_flg & IPV6_FW_F_ICMPBIT) {
-		int type_index;
+		size_t type_index;
 		int first = 1;
 
 		printf(" icmptype");
@@ -432,19 +427,20 @@
 		endservent();
 }
 
-void
+static void
 list(int ac, char **av)
 {
 	struct ip6_fw *r, *rules, *n;
-	int l,i;
+	int i;
+	size_t l;
 	unsigned long rulenum;
 	int nalloc, bytes, maxbytes;
 
 	/* extract rules from kernel, resizing array as necessary */
 	rules = NULL;
-	nalloc = sizeof *rules;
+	nalloc = sizeof(*rules);
 	bytes = nalloc;
-	maxbytes = 65536 * sizeof *rules;
+	maxbytes = 65536 * sizeof(*rules);
 	while (bytes >= nalloc) {
 		if ((n = realloc(rules, nalloc * 2 + 200)) == NULL)
 			err(EX_OSERR, "realloc");
@@ -456,11 +452,10 @@
 	}
 	if (!ac) {
 		/* display all rules */
-		for (r = rules, l = bytes; l >= sizeof rules[0];
-			 r++, l-=sizeof rules[0])
+		for (r = rules, l = bytes; l >= sizeof(rules[0]);
+			 r++, l-=sizeof(rules[0]))
 			show_ip6fw(r);
-	}
-	else {
+	} else {
 		/* display specific rules requested on command line */
 		int exitval = 0;
 
@@ -545,13 +540,13 @@
 	return(0);
 }
 
-void
+static void
 fill_ip6(struct in6_addr *ipno, struct in6_addr *mask, int *acp, char ***avp)
 {
 	int ac = *acp;
 	char **av = *avp;
 	char *p = 0, md = 0;
-	int i;
+	size_t i;
 
 	if (ac && !strncmp(*av,"any",strlen(*av))) {
 		*ipno = *mask = in6addr_any; av++; ac--;
@@ -592,10 +587,10 @@
 {
 	struct icmpcode *ic;
 	u_long val;
-	char *s;
+	char *ep;
 
-	val = strtoul(str, &s, 0);
-	if (s != str && *s == '\0' && val < 0x100) {
+	val = strtoul(str, &ep, 0);
+	if (ep != str && *ep == '\0' && val < 0x100) {
 		*codep = val;
 		return;
 	}
@@ -621,15 +616,15 @@
 {
 	int		val;
 	char		*earg, buf[32];
-	struct servent	*s;
 
 	snprintf(buf, sizeof(buf), "%s", arg);
 	buf[strcspn(arg, nodash ? "-," : ",")] = 0;
 	val = (int) strtoul(buf, &earg, 0);
 	if (!*buf || *earg) {
+		struct servent	*sv;
 		setservent(1);
-		if ((s = getservbyname(buf, NULL))) {
-			val = htons(s->s_port);
+		if ((sv = getservbyname(buf, NULL))) {
+			val = htons(sv->s_port);
 		} else {
 			if (!test) {
 				errx(1, "unknown port ``%s''", arg);
@@ -647,37 +642,37 @@
 	return(val);
 }
 
-int
+static int
 fill_port(u_short *cnt, u_short *ptr, u_short off, char *arg)
 {
-	char *s;
+	char *p;
 	int initial_range = 0;
 
-	s = arg + strcspn(arg, "-,");	/* first port name can't have a dash */
-	if (*s == '-') {
-		*s++ = '\0';
+	p = arg + strcspn(arg, "-,");	/* first port name can't have a dash */
+	if (*p == '-') {
+		*p++ = '\0';
 		if (strchr(arg, ','))
 			errx(1, "port range must be first in list");
 		add_port(cnt, ptr, off, *arg ? lookup_port(arg, 0, 0) : 0x0000);
-		arg = s;
-		s = strchr(arg,',');
-		if (s)
-			*s++ = '\0';
+		arg = p;
+		p = strchr(arg,',');
+		if (p)
+			*p++ = '\0';
 		add_port(cnt, ptr, off, *arg ? lookup_port(arg, 0, 0) : 0xffff);
-		arg = s;
+		arg = p;
 		initial_range = 1;
 	}
 	while (arg != NULL) {
-		s = strchr(arg,',');
-		if (s)
-			*s++ = '\0';
+		p = strchr(arg,',');
+		if (p)
+			*p++ = '\0';
 		add_port(cnt, ptr, off, lookup_port(arg, 0, 0));
-		arg = s;
+		arg = p;
 	}
 	return initial_range;
 }
 
-void
+static void
 fill_tcpflag(u_char *set, u_char *reset, char **vp)
 {
 	char *p = *vp,*q;
@@ -685,7 +680,7 @@
 
 	while (p && *p) {
 		struct tpcflags {
-			char * name;
+			const char * name;
 			u_char value;
 		} flags[] = {
 			{ "syn", IPV6_FW_TCPF_SYN },
@@ -695,7 +690,7 @@
 			{ "rst", IPV6_FW_TCPF_RST },
 			{ "urg", IPV6_FW_TCPF_URG }
 		};
-		int i;
+		size_t i;
 
 		if (*p == '!') {
 			p++;
@@ -744,7 +739,7 @@
 	}
 }
 
-void
+static void
 fill_icmptypes(unsigned *types, char **vp, u_short *fw_flg)
 {
 	char *c = *vp;
@@ -770,11 +765,10 @@
 	}
 }
 
-void
+static void
 delete(int ac, char **av)
 {
 	struct ip6_fw rule;
-	int i;
 	int exitval = 0;
 
 	memset(&rule, 0, sizeof rule);
@@ -784,10 +778,10 @@
 	/* Rule number */
 	while (ac && isdigit(**av)) {
 		rule.fw_number = atoi(*av); av++; ac--;
-		i = setsockopt(s, IPPROTO_IPV6, IPV6_FW_DEL, &rule, sizeof rule);
-		if (i) {
+		if (setsockopt(s, IPPROTO_IPV6, IPV6_FW_DEL,
+			       &rule, sizeof(rule)) < 0) {
 			exitval = 1;
-			warn("rule %u: setsockopt(%s)", rule.fw_number, "IPV6_FW_DEL");
+			warn("rule %u: setsockopt(IPV6_FW_DEL)", rule.fw_number);
 		}
 	}
 	if (exitval != 0)
@@ -806,7 +800,8 @@
 }
 
 static void
-fill_iface(char *which, union ip6_fw_if *ifu, int *byname, int ac, char *arg)
+fill_iface(const char *which, union ip6_fw_if *ifu, int *byname,
+	   int ac, char *arg)
 {
 	if (!ac)
 	    show_usage("missing argument for ``%s''", which);
@@ -838,7 +833,6 @@
 add(int ac, char **av)
 {
 	struct ip6_fw rule;
-	int i;
 	u_char proto;
 	struct protoent *pe;
 	int saw_xmrc = 0, saw_via = 0;
@@ -1117,9 +1111,8 @@
 
 	if (!do_quiet)
 		show_ip6fw(&rule);
-	i = setsockopt(s, IPPROTO_IPV6, IPV6_FW_ADD, &rule, sizeof rule);
-	if (i)
-		err(EX_UNAVAILABLE, "setsockopt(%s)", "IPV6_FW_ADD");
+	if (setsockopt(s, IPPROTO_IPV6, IPV6_FW_ADD, &rule, sizeof rule) < 0)
+		err(EX_UNAVAILABLE, "setsockopt(IPV6_FW_ADD)");
 }
 
 static void
@@ -1130,7 +1123,7 @@
 	if (!ac) {
 		/* clear all entries */
 		if (setsockopt(s,IPPROTO_IPV6,IPV6_FW_ZERO,NULL,0)<0)
-			err(EX_UNAVAILABLE, "setsockopt(%s)", "IPV6_FW_ZERO");
+			err(EX_UNAVAILABLE, "setsockopt(IPV6_FW_ZERO)");
 		if (!do_quiet)
 			printf("Accounting cleared.\n");
 	} else {
@@ -1144,11 +1137,9 @@
 				rule.fw_number = atoi(*av); av++; ac--;
 				if (setsockopt(s, IPPROTO_IPV6,
 				    IPV6_FW_ZERO, &rule, sizeof rule)) {
-					warn("rule %u: setsockopt(%s)", rule.fw_number,
-						 "IPV6_FW_ZERO");
+					warn("rule %u: setsockopt(IPV6_FW_ZERO)", rule.fw_number);
 					failed = 1;
-				}
-				else if (!do_quiet)
+				} else if (!do_quiet)
 					printf("Entry %d cleared\n",
 					    rule.fw_number);
 			} else
@@ -1159,11 +1150,10 @@
 	}
 }
 
-int
+static int
 ip6fw_main(int ac, char **av)
 {
 	int 		ch;
-	extern int 	optind;
 
 	/* init optind to 1 */
 	optind = 1;
@@ -1228,7 +1218,7 @@
 		}
 		if ( do_flush ) {
 			if (setsockopt(s,IPPROTO_IPV6,IPV6_FW_FLUSH,NULL,0) < 0)
-				err(EX_UNAVAILABLE, "setsockopt(%s)", "IPV6_FW_FLUSH");
+				err(EX_UNAVAILABLE, "setsockopt(IPV6_FW_FLUSH)");
 			if (!do_quiet)
 				printf("Flushed all rules.\n");
 		}
@@ -1254,7 +1244,7 @@
 #define	WHITESP		" \t\f\v\n\r"
 	char	buf[BUFSIZ];
 	char	*a, *p, *args[MAX_ARGS], *cmd = NULL;
-	char	linename[10];
+	char	linename[10], q_opt[3];
 	int 	i, c, lineno, qflag, pflag, status;
 	FILE	*f = NULL;
 	pid_t	preproc = 0;
@@ -1270,6 +1260,7 @@
 	 * be preprocessed if it is specified as an absolute pathname.
 	 */
 
+	strcpy(q_opt, "-q");
 	if (ac > 1 && av[ac - 1][0] == '/' && access(av[ac - 1], R_OK) == 0) {
 		qflag = pflag = i = 0;
 		lineno = 0;
@@ -1282,7 +1273,7 @@
 				if (i > MAX_ARGS - 2)
 					errx(EX_USAGE,
 					     "too many -D or -U options");
-				args[i++] = "-D";
+				args[i++] = av[optind - 2];
 				args[i++] = optarg;
 				break;
 
@@ -1292,7 +1283,7 @@
 				if (i > MAX_ARGS - 2)
 					errx(EX_USAGE,
 					     "too many -D or -U options");
-				args[i++] = "-U";
+				args[i++] = av[optind - 2];
 				args[i++] = optarg;
 				break;
 
@@ -1367,7 +1358,8 @@
 			if ((p = strchr(buf, '#')) != NULL)
 				*p = '\0';
 			i=1;
-			if (qflag) args[i++]="-q";
+			if (qflag)
+				args[i++] = q_opt;
 			for (a = strtok(buf, WHITESP);
 			    a && i < MAX_ARGS; a = strtok(NULL, WHITESP), i++)
 				args[i] = a;