[BusyBox] bug#1136: option -Wshadow

Larry Doolittle ldoolitt at recycle.lbl.gov
Wed Mar 21 01:02:47 UTC 2001


Package: busybox
Version: March
Severity: wishlist

The following patch adds -Wshadow to the make conditions,
and "fixes" most of the compiler complaints that were lurking
in the code base.  After these easy shadows are cleaned up,
you can clearly see the few truly scary cases, like nfsmount
and insmod.  I didn't fix tr.c, because I think that shadow
will disappear when someone getopt's it.

       - Larry Doolittle   <LRDoolittle at lbl.gov>

diff -ur /home/ldoolitt/cvs/busybox/Makefile busybox-trial/Makefile
--- /home/ldoolitt/cvs/busybox/Makefile	Mon Mar 19 11:34:04 2001
+++ busybox-trial/Makefile	Tue Mar 20 16:02:23 2001
@@ -108,7 +108,7 @@
 OPTIMIZATION := $(shell if $(CC) -Os -S -o /dev/null -xc /dev/null >/dev/null 2>&1; \
     then echo "-Os"; else echo "-O2" ; fi)
 
-WARNINGS = -Wall
+WARNINGS = -Wall -Wshadow
 
 ARFLAGS = -r
 
Only in busybox-trial: busybox
diff -ur /home/ldoolitt/cvs/busybox/dd.c busybox-trial/dd.c
--- /home/ldoolitt/cvs/busybox/dd.c	Tue Feb 20 08:40:18 2001
+++ busybox-trial/dd.c	Tue Mar 20 16:36:13 2001
@@ -45,7 +45,7 @@
 
 int dd_main(int argc, char **argv)
 {
-	int i, ifd, ofd, oflag, sync = FALSE, trunc = TRUE;
+	int i, ifd, ofd, oflag, sync_flag = FALSE, trunc = TRUE;
 	size_t in_full = 0, in_part = 0, out_full = 0, out_part = 0;
 	size_t bs = 512, count = -1;
 	ssize_t n;
@@ -73,7 +73,7 @@
 					trunc = FALSE;
 					buf += 7;
 				} else if (strncmp("sync", buf, 4) == 0) {
-					sync = TRUE;
+					sync_flag = TRUE;
 					buf += 4;
 				} else {
 					error_msg_and_die("invalid conversion `%s'", argv[i]+5);
@@ -138,7 +138,7 @@
 			in_full++;
 		else
 			in_part++;
-		if (sync) {
+		if (sync_flag) {
 			memset(buf + n, '\0', bs - n);
 			n = bs;
 		}
diff -ur /home/ldoolitt/cvs/busybox/echo.c busybox-trial/echo.c
--- /home/ldoolitt/cvs/busybox/echo.c	Tue Feb 20 08:40:19 2001
+++ busybox-trial/echo.c	Tue Mar 20 16:54:52 2001
@@ -40,7 +40,7 @@
 	while (argc > 0 && *argv[0] == '-')
 	{
 		register char *temp;
-		register int index;
+		register int ix;
 
 		/*
 		 * If it appears that we are handling options, then make sure
@@ -49,9 +49,9 @@
 		 */
 		temp = argv[0] + 1;
 
-		for (index = 0; temp[index]; index++)
+		for (ix = 0; temp[ix]; ix++)
 		{
-			if (strrchr("neE", temp[index]) == 0)
+			if (strrchr("neE", temp[ix]) == 0)
 				goto just_echo;
 		}
 
diff -ur /home/ldoolitt/cvs/busybox/fsck_minix.c busybox-trial/fsck_minix.c
--- /home/ldoolitt/cvs/busybox/fsck_minix.c	Fri Mar  9 14:09:18 2001
+++ busybox-trial/fsck_minix.c	Tue Mar 20 16:51:07 2001
@@ -1439,18 +1439,18 @@
 		check();
 	}
 	if (verbose) {
-		int i, free;
+		int i, free_cnt;
 
-		for (i = 1, free = 0; i <= INODES; i++)
+		for (i = 1, free_cnt = 0; i <= INODES; i++)
 			if (!inode_in_use(i))
-				free++;
-		printf("\n%6ld inodes used (%ld%%)\n", (INODES - free),
-			   100 * (INODES - free) / INODES);
-		for (i = FIRSTZONE, free = 0; i < ZONES; i++)
+				free_cnt++;
+		printf("\n%6ld inodes used (%ld%%)\n", (INODES - free_cnt),
+			   100 * (INODES - free_cnt) / INODES);
+		for (i = FIRSTZONE, free_cnt = 0; i < ZONES; i++)
 			if (!zone_in_use(i))
-				free++;
-		printf("%6ld zones used (%ld%%)\n", (ZONES - free),
-			   100 * (ZONES - free) / ZONES);
+				free_cnt++;
+		printf("%6ld zones used (%ld%%)\n", (ZONES - free_cnt),
+			   100 * (ZONES - free_cnt) / ZONES);
 		printf("\n%6d regular files\n"
 			   "%6d directories\n"
 			   "%6d character device files\n"
diff -ur /home/ldoolitt/cvs/busybox/init.c busybox-trial/init.c
--- /home/ldoolitt/cvs/busybox/init.c	Tue Mar 20 09:40:06 2001
+++ busybox-trial/init.c	Tue Mar 20 16:22:43 2001
@@ -512,7 +512,6 @@
 		 */
 
 		if (*cmdpath == '-') {
-			char *s;
 
 			/* skip over the dash */
 			++cmdpath;
diff -ur /home/ldoolitt/cvs/busybox/libbb/parse_mode.c busybox-trial/libbb/parse_mode.c
--- /home/ldoolitt/cvs/busybox/libbb/parse_mode.c	Fri Mar 16 14:47:14 2001
+++ busybox-trial/libbb/parse_mode.c	Tue Mar 20 16:32:23 2001
@@ -50,8 +50,8 @@
 		S_ISVTX					/* t */
 	};
 
-	static const char group_string[] = "ugoa";
-	static const char mode_string[] = "rwxst";
+	static const char group_chars[] = "ugoa";
+	static const char mode_chars[] = "rwxst";
 
 	const char *p;
 
@@ -74,9 +74,9 @@
 		if ((c = *s++) == '\0') {
 			return -1;
 		}
-		for (p=group_string ; *p ; p++) {
+		for (p=group_chars ; *p ; p++) {
 			if (*p == c) {
-				groups |= group_set[(int)(p-group_string)];
+				groups |= group_set[(int)(p-group_chars)];
 				goto NEXT_GROUP;
 			}
 		}
@@ -101,9 +101,9 @@
 
 	NEXT_MODE:
 		if (((c = *s++) != '\0') && (c != ',')) {
-			for (p=mode_string ; *p ; p++) {
+			for (p=mode_chars ; *p ; p++) {
 				if (*p == c) {
-					mode |= mode_set[(int)(p-mode_string)];
+					mode |= mode_set[(int)(p-mode_chars)];
 					goto NEXT_MODE;
 				}
 			}
diff -ur /home/ldoolitt/cvs/busybox/libbb/recursive_action.c busybox-trial/libbb/recursive_action.c
--- /home/ldoolitt/cvs/busybox/libbb/recursive_action.c	Fri Mar 16 14:47:14 2001
+++ busybox-trial/libbb/recursive_action.c	Tue Mar 20 16:52:22 2001
@@ -26,6 +26,7 @@
  */
 
 #include <stdio.h>
+#include <string.h>
 #include <dirent.h>
 #include <sys/stat.h>
 #include "libbb.h"
diff -ur /home/ldoolitt/cvs/busybox/nfsmount.c busybox-trial/nfsmount.c
--- /home/ldoolitt/cvs/busybox/nfsmount.c	Fri Mar  9 14:09:18 2001
+++ busybox-trial/nfsmount.c	Tue Mar 20 16:25:01 2001
@@ -157,7 +157,7 @@
 #define HAVE_personality
 #define HAVE_tm_gmtoff
 
-static char *nfs_strerror(int stat);
+static char *nfs_strerror(int status);
 
 #define MAKE_VERSION(p,q,r)	(65536*(p) + 256*(q) + (r))
 #define MAX_NFSPROT ((nfs_mount_version >= 4) ? 3 : 2)
@@ -873,16 +873,16 @@
 	{ -1,			EIO		}
 };
 
-static char *nfs_strerror(int stat)
+static char *nfs_strerror(int status)
 {
 	int i;
 	static char buf[256];
 
 	for (i = 0; nfs_errtbl[i].stat != -1; i++) {
-		if (nfs_errtbl[i].stat == stat)
+		if (nfs_errtbl[i].stat == status)
 			return strerror(nfs_errtbl[i].errnum);
 	}
-	sprintf(buf, _("unknown nfs status return value: %d"), stat);
+	sprintf(buf, _("unknown nfs status return value: %d"), status);
 	return buf;
 }
 
diff -ur /home/ldoolitt/cvs/busybox/rdate.c busybox-trial/rdate.c
--- /home/ldoolitt/cvs/busybox/rdate.c	Tue Mar 20 09:40:06 2001
+++ busybox-trial/rdate.c	Tue Mar 20 16:46:20 2001
@@ -40,7 +40,7 @@
 static time_t askremotedate(const char *host)
 {
 	struct hostent *h;
-	struct sockaddr_in sin;
+	struct sockaddr_in s_in;
 	struct servent *tserv;
 	unsigned long int nett, localt;
 	int fd;
@@ -54,11 +54,11 @@
 	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0)    /* get net connection */
 		perror_msg_and_die("%s", "socket");
 
-	memcpy(&sin.sin_addr, h->h_addr, sizeof(sin.sin_addr));
-	sin.sin_port= tserv->s_port;
-	sin.sin_family = AF_INET;
+	memcpy(&s_in.sin_addr, h->h_addr, sizeof(s_in.sin_addr));
+	s_in.sin_port= tserv->s_port;
+	s_in.sin_family = AF_INET;
 
-	if (connect(fd, (struct sockaddr *)&sin, sizeof(sin)) < 0)      /* connect to time server */
+	if (connect(fd, (struct sockaddr *)&s_in, sizeof(s_in)) < 0)      /* connect to time server */
 		perror_msg_and_die("%s", host);
 
 	if (read(fd, (void *)&nett, 4) != 4)    /* read time from server */
@@ -79,7 +79,7 @@
 
 int rdate_main(int argc, char **argv)
 {
-	time_t time;
+	time_t remote_time;
 	int opt;
 	int setdate = 0;
 	int printdate= 0;
@@ -111,15 +111,15 @@
 	if (optind == argc)
 		show_usage();
 
-	time = askremotedate(argv[optind]);
+	remote_time = askremotedate(argv[optind]);
 
 	if (setdate) {
-		if (stime(&time) < 0)
+		if (stime(&remote_time) < 0)
 			perror_msg_and_die("Could not set time of day");
 	}
 
 	if (printdate)
-		printf("%s", ctime(&time));
+		printf("%s", ctime(&remote_time));
 
 	return EXIT_SUCCESS;
 }
diff -ur /home/ldoolitt/cvs/busybox/route.c busybox-trial/route.c
--- /home/ldoolitt/cvs/busybox/route.c	Fri Mar  9 14:09:18 2001
+++ busybox-trial/route.c	Tue Mar 20 16:44:02 2001
@@ -60,18 +60,18 @@
 static int
 INET_resolve(char *name, struct sockaddr *sa)
 {
-	struct sockaddr_in *sin = (struct sockaddr_in *)sa;
+	struct sockaddr_in *s_in = (struct sockaddr_in *)sa;
 	
-	sin->sin_family = AF_INET;
-	sin->sin_port = 0;
+	s_in->sin_family = AF_INET;
+	s_in->sin_port = 0;
 
 	/* Default is special, meaning 0.0.0.0. */
 	if (strcmp(name, "default")==0) {
-		sin->sin_addr.s_addr = INADDR_ANY;
+		s_in->sin_addr.s_addr = INADDR_ANY;
 		return 1;
 	}
 	/* Look to see if it's a dotted quad. */
-	if (inet_aton(name, &sin->sin_addr)) {
+	if (inet_aton(name, &s_in->sin_addr)) {
 		return 0;
 	}
 	/* guess not.. */
diff -ur /home/ldoolitt/cvs/busybox/sh.c busybox-trial/sh.c
--- /home/ldoolitt/cvs/busybox/sh.c	Fri Mar 16 16:20:24 2001
+++ busybox-trial/sh.c	Tue Mar 20 16:20:12 2001
@@ -687,16 +687,16 @@
 	cmd->job_list = keep;
 }
 
-/* remove a job from the job_list */
-static void remove_job(struct jobset *job_list, struct job *job)
+/* remove a job from a jobset */
+static void remove_job(struct jobset *j_list, struct job *job)
 {
 	struct job *prevjob;
 
 	free_job(job);
-	if (job == job_list->head) {
-		job_list->head = job->next;
+	if (job == j_list->head) {
+		j_list->head = job->next;
 	} else {
-		prevjob = job_list->head;
+		prevjob = j_list->head;
 		while (prevjob->next != job)
 			prevjob = prevjob->next;
 		prevjob->next = job->next;
@@ -707,7 +707,7 @@
 
 /* Checks to see if any background processes have exited -- if they 
    have, figure out why and see if a job has completed */
-static void checkjobs(struct jobset *job_list)
+static void checkjobs(struct jobset *j_list)
 {
 	struct job *job;
 	pid_t childpid;
@@ -715,7 +715,7 @@
 	int prognum = 0;
 
 	while ((childpid = waitpid(-1, &status, WNOHANG | WUNTRACED)) > 0) {
-		for (job = job_list->head; job; job = job->next) {
+		for (job = j_list->head; job; job = job->next) {
 			prognum = 0;
 			while (prognum < job->num_progs &&
 				   job->progs[prognum].pid != childpid) prognum++;
@@ -734,7 +734,7 @@
 
 			if (!job->running_progs) {
 				printf(JOB_STATUS_FORMAT, job->jobid, "Done", job->text);
-				remove_job(job_list, job);
+				remove_job(j_list, job);
 			}
 		} else {
 			/* child stopped */
@@ -907,35 +907,35 @@
 #endif	
 
 #if defined BB_FEATURE_SH_ENVIRONMENT && ! defined BB_FEATURE_SH_WORDEXP
-char * strsep_space( char *string, int * index)
+char * strsep_space( char *string, int * ix)
 {
 	char *token, *begin;
 
 	begin = string;
 
 	/* Short circuit the trivial case */
-	if ( !string || ! string[*index])
+	if ( !string || ! string[*ix])
 		return NULL;
 
 	/* Find the end of the token. */
-	while( string && string[*index] && !isspace(string[*index]) ) {
-		(*index)++;
+	while( string && string[*ix] && !isspace(string[*ix]) ) {
+		(*ix)++;
 	}
 
 	/* Find the end of any whitespace trailing behind 
 	 * the token and let that be part of the token */
-	while( string && string[*index] && isspace(string[*index]) ) {
-		(*index)++;
+	while( string && string[*ix] && isspace(string[*ix]) ) {
+		(*ix)++;
 	}
 
-	if (! string && *index==0) {
+	if (! string && *ix==0) {
 		/* Nothing useful was found */
 		return NULL;
 	}
 
-	token = xmalloc(*index+1);
-	token[*index] = '\0';
-	strncpy(token, string,  *index); 
+	token = xmalloc(*ix+1);
+	token[*ix] = '\0';
+	strncpy(token, string,  *ix); 
 
 	return token;
 }
@@ -947,7 +947,7 @@
 #ifdef BB_FEATURE_SH_ENVIRONMENT
 	expand_t expand_result;
 	char *src, *dst, *var;
-	int index = 0;
+	int ix = 0;
 	int i=0, length, total_length=0, retval;
 	const char *out_of_space = "out of space during expansion"; 
 #endif
@@ -956,13 +956,13 @@
 	chomp(command);
 	
 	/* Fix up escape sequences to be the Real Thing(tm) */
-	while( command && command[index]) {
-		if (command[index] == '\\') {
-			char *tmp = command+index+1;
-			command[index] = process_escape_sequence(  &tmp );
-			memmove(command+index + 1, tmp, strlen(tmp)+1);
+	while( command && command[ix]) {
+		if (command[ix] == '\\') {
+			char *tmp = command+ix+1;
+			command[ix] = process_escape_sequence(  &tmp );
+			memmove(command+ix + 1, tmp, strlen(tmp)+1);
 		}
-		index++;
+		ix++;
 	}
 
 #ifdef BB_FEATURE_SH_ENVIRONMENT
@@ -1025,8 +1025,8 @@
 		 * we write stuff into the original (in a minute) */
 		cmd = cmd_copy = strdup(command);
 		*command = '\0';
-		for (index = 0, tmpcmd = cmd; 
-				(tmpcmd = strsep_space(cmd, &index)) != NULL; cmd += index, index=0) {
+		for (ix = 0, tmpcmd = cmd; 
+				(tmpcmd = strsep_space(cmd, &ix)) != NULL; cmd += ix, ix=0) {
 			if (*tmpcmd == '\0')
 				break;
 			retval = glob(tmpcmd, flags, NULL, &expand_result);
@@ -1096,11 +1096,11 @@
 			case '0':case '1':case '2':case '3':case '4':
 			case '5':case '6':case '7':case '8':case '9':
 				{
-					int index=*(dst + 1)-48;
-					if (index >= argc) {
+					int ixx=*(dst + 1)-48;
+					if (ixx >= argc) {
 						var='\0';
 					} else {
-						var = argv[index];
+						var = argv[ixx];
 					}
 				}
 				break;
@@ -1575,19 +1575,19 @@
 static void insert_job(struct job *newjob, int inbg)
 {
 	struct job *thejob;
-	struct jobset *job_list=newjob->job_list;
+	struct jobset *j_list=newjob->job_list;
 
 	/* find the ID for thejob to use */
 	newjob->jobid = 1;
-	for (thejob = job_list->head; thejob; thejob = thejob->next)
+	for (thejob = j_list->head; thejob; thejob = thejob->next)
 		if (thejob->jobid >= newjob->jobid)
 			newjob->jobid = thejob->jobid + 1;
 
 	/* add thejob to the list of running jobs */
-	if (!job_list->head) {
-		thejob = job_list->head = xmalloc(sizeof(*thejob));
+	if (!j_list->head) {
+		thejob = j_list->head = xmalloc(sizeof(*thejob));
 	} else {
-		for (thejob = job_list->head; thejob->next; thejob = thejob->next) /* nothing */;
+		for (thejob = j_list->head; thejob->next; thejob = thejob->next) /* nothing */;
 		thejob->next = xmalloc(sizeof(*thejob));
 		thejob = thejob->next;
 	}
diff -ur /home/ldoolitt/cvs/busybox/syslogd.c busybox-trial/syslogd.c
--- /home/ldoolitt/cvs/busybox/syslogd.c	Tue Mar 13 09:08:07 2001
+++ busybox-trial/syslogd.c	Tue Mar 20 16:12:52 2001
@@ -477,8 +477,6 @@
 	int sock_fd;
 	fd_set fds;
 
-	RESERVE_BB_BUFFER(lfile, BUFSIZ);
-
 	/* Set up signal handlers. */
 	signal (SIGINT,  quit_signal);
 	signal (SIGTERM, quit_signal);
diff -ur /home/ldoolitt/cvs/busybox/tr.c busybox-trial/tr.c
--- /home/ldoolitt/cvs/busybox/tr.c	Tue Feb 20 08:40:19 2001
+++ busybox-trial/tr.c	Tue Mar 20 16:43:35 2001
@@ -123,19 +123,19 @@
 
 static int complement(unsigned char *buffer, int buffer_len)
 {
-	register short i, j, index;
+	register short i, j, ix;
 	char conv[ASCII + 2];
 
-	index = 0;
+	ix = 0;
 	for (i = 0; i <= ASCII; i++) {
 		for (j = 0; j < buffer_len; j++)
 			if (buffer[j] == i)
 				break;
 		if (j == buffer_len)
-			conv[index++] = i & ASCII;
+			conv[ix++] = i & ASCII;
 	}
-	memcpy(buffer, conv, index);
-	return index;
+	memcpy(buffer, conv, ix);
+	return ix;
 }
 
 extern int tr_main(int argc, char **argv)
diff -ur /home/ldoolitt/cvs/busybox/umount.c busybox-trial/umount.c
--- /home/ldoolitt/cvs/busybox/umount.c	Tue Feb 20 08:40:19 2001
+++ busybox-trial/umount.c	Tue Mar 20 16:09:20 2001
@@ -57,7 +57,9 @@
 #if defined BB_FEATURE_MOUNT_LOOP
 static int freeLoop = TRUE;
 #endif
+#if defined BB_MTAB
 static int useMtab = TRUE;
+#endif
 static int umountAll = FALSE;
 static int doRemount = FALSE;
 extern const char mtab_file[];	/* Defined in utility.c */
@@ -162,7 +164,7 @@
 }
 #endif
 
-static int do_umount(const char *name, int useMtab)
+static int do_umount(const char *name)
 {
 	int status;
 	char *blockDevice = mtab_getinfo(name, MTAB_GETDEVICE);
@@ -204,7 +206,7 @@
 	return (FALSE);
 }
 
-static int umount_all(int useMtab)
+static int umount_all(void)
 {
 	int status = TRUE;
 	char *mountpt;
@@ -214,14 +216,14 @@
 		/* Never umount /proc on a umount -a */
 		if (strstr(mountpt, "proc")!= NULL)
 			continue;
-		if (!do_umount(mountpt, useMtab)) {
+		if (!do_umount(mountpt)) {
 			/* Don't bother retrying the umount on busy devices */
 			if (errno == EBUSY) {
 				perror_msg("%s", mountpt);
 				status = FALSE;
 				continue;
 			}
-			if (!do_umount(mountpt, useMtab)) {
+			if (!do_umount(mountpt)) {
 				printf("Couldn't umount %s on %s: %s\n",
 					   mountpt, mtab_getinfo(mountpt, MTAB_GETDEVICE),
 					   strerror(errno));
@@ -275,12 +277,12 @@
 
 	mtab_read();
 	if (umountAll == TRUE) {
-		if (umount_all(useMtab) == TRUE)
+		if (umount_all() == TRUE)
 			return EXIT_SUCCESS;
 		else
 			return EXIT_FAILURE;
 	}
-	if (do_umount(*argv, useMtab) == TRUE)
+	if (do_umount(*argv) == TRUE)
 		return EXIT_SUCCESS;
 	perror_msg_and_die("%s", *argv);
 }
diff -ur /home/ldoolitt/cvs/busybox/wget.c busybox-trial/wget.c
--- /home/ldoolitt/cvs/busybox/wget.c	Fri Mar  9 14:09:18 2001
+++ busybox-trial/wget.c	Tue Mar 20 16:41:17 2001
@@ -291,24 +291,24 @@
 
 FILE *open_socket(char *host, int port)
 {
-	struct sockaddr_in sin;
+	struct sockaddr_in s_in;
 	struct hostent *hp;
 	int fd;
 	FILE *fp;
 
-	memset(&sin, 0, sizeof(sin));
-	sin.sin_family = AF_INET;
+	memset(&s_in, 0, sizeof(s_in));
+	s_in.sin_family = AF_INET;
 	if ((hp = (struct hostent *) gethostbyname(host)) == NULL)
 		error_msg_and_die("cannot resolve %s", host);
-	memcpy(&sin.sin_addr, hp->h_addr_list[0], hp->h_length);
-	sin.sin_port = htons(port);
+	memcpy(&s_in.sin_addr, hp->h_addr_list[0], hp->h_length);
+	s_in.sin_port = htons(port);
 
 	/*
 	 * Get the server onto a stdio stream.
 	 */
 	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0)
 		perror_msg_and_die("socket()");
-	if (connect(fd, (struct sockaddr *) &sin, sizeof(sin)) < 0)
+	if (connect(fd, (struct sockaddr *) &s_in, sizeof(s_in)) < 0)
 		perror_msg_and_die("connect(%s)", host);
 	if ((fp = fdopen(fd, "r+")) == NULL)
 		perror_msg_and_die("fdopen()");






More information about the busybox mailing list