svn commit: trunk/busybox: libbb procps

[vda at busybox.net]

Author: vda
Date: 2008-08-19 17:12:22 -0700 (Tue, 19 Aug 2008)
New Revision: 23111

Log:
libbb: fix mishandling of "all argv are opts" in getopt32()

function                                             old     new   delta
top_main                                            1100    1095      -5
getopt32                                            1398    1361     -37
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-42)             Total: -42 bytes



Modified:
   trunk/busybox/libbb/getopt32.c
   trunk/busybox/procps/top.c


Changeset:
Modified: trunk/busybox/libbb/getopt32.c
===================================================================
--- trunk/busybox/libbb/getopt32.c	2008-08-19 23:02:23 UTC (rev 23110)
+++ trunk/busybox/libbb/getopt32.c	2008-08-20 00:12:22 UTC (rev 23111)
@@ -174,7 +174,7 @@
         on the command line.
 
  "V-"   An option with dash before colon or end-of-line results in
-        bb_show_usage being called if this option is encountered.
+        bb_show_usage() being called if this option is encountered.
         This is typically used to implement "print verbose usage message
         and exit" option.
 
@@ -285,10 +285,6 @@
 
 const char *opt_complementary;
 
-/* Many small applets don't want to suck in stdio.h only because
- * they need to parse options by calling us */
-#define DONT_USE_PRINTF 1
-
 enum {
 	PARAM_STRING,
 	PARAM_LIST,
@@ -322,7 +318,7 @@
 	int argc;
 	unsigned flags = 0;
 	unsigned requires = 0;
-	t_complementary complementary[33];
+	t_complementary complementary[33]; /* last stays zero-filled */
 	int c;
 	const unsigned char *s;
 	t_complementary *on_off;
@@ -332,14 +328,13 @@
 	struct option *long_options = (struct option *) &bb_null_long_options;
 #endif
 	unsigned trigger;
-	char **pargv = NULL;
+	char **pargv;
 	int min_arg = 0;
 	int max_arg = -1;
 
 #define SHOW_USAGE_IF_ERROR     1
 #define ALL_ARGV_IS_OPTS        2
 #define FIRST_ARGV_IS_OPT       4
-#define FREE_FIRST_ARGV_IS_OPT  (8 * !DONT_USE_PRINTF)
 
 	int spec_flgs = 0;
 
@@ -493,17 +488,18 @@
 	}
 	va_end(p);
 
-	if (spec_flgs & FIRST_ARGV_IS_OPT) {
-		if (argv[1] && argv[1][0] != '-' && argv[1][0] != '\0') {
-#if DONT_USE_PRINTF
-			char *pp = alloca(strlen(argv[1]) + 2);
-			*pp = '-';
-			strcpy(pp + 1, argv[1]);
-			argv[1] = pp;
-#else
-			argv[1] = xasprintf("-%s", argv[1]);
-			spec_flgs |= FREE_FIRST_ARGV_IS_OPT;
-#endif
+	if (spec_flgs & (FIRST_ARGV_IS_OPT | ALL_ARGV_IS_OPTS)) {
+		pargv = argv + 1;
+		while (*pargv) {
+			if (pargv[0][0] != '-' && pargv[0][0] != '\0') {
+				char *pp = alloca(strlen(*pargv) + 2);
+				*pp = '-';
+				strcpy(pp + 1, *pargv);
+				*pargv = pp;
+			}
+			if (!(spec_flgs & ALL_ARGV_IS_OPTS))
+				break; 
+			pargv++;
 		}
 	}
 
@@ -529,6 +525,7 @@
 	/* optreset = 1; */
 #endif
 	/* optarg = NULL; opterr = 0; optopt = 0; - do we need this?? */
+	pargv = NULL;
 
 	/* Note: just "getopt() <= 0" will not work well for
 	 * "fake" short options, like this one:
@@ -540,12 +537,17 @@
 #else
 	while ((c = getopt(argc, argv, applet_opts)) != -1) {
 #endif
+		/* getopt prints "option requires an argument -- X"
+		 * and returns '?' if an option has no arg, but one is reqd */
 		c &= 0xff; /* fight libc's sign extension */
- loop_arg_is_opt:
 		for (on_off = complementary; on_off->opt_char != c; on_off++) {
-			/* c==0 if long opt have non NULL flag */
-			if (on_off->opt_char == '\0' && c != '\0')
+			/* c can be NUL if long opt has non-NULL ->flag,
+			 * but we construct long opts so that flag
+			 * is always NULL (see above) */
+			if (on_off->opt_char == '\0' /* && c != '\0' */) {
+				/* c is probably '?' - "bad option" */
 				bb_show_usage();
+			}
 		}
 		if (flags & on_off->incongruously)
 			bb_show_usage();
@@ -570,24 +572,6 @@
 			break;
 	}
 
-	if (spec_flgs & ALL_ARGV_IS_OPTS) {
-		/* process argv is option, for example "ps" applet */
-		if (pargv == NULL)
-			pargv = argv + optind;
-		while (*pargv) {
-			c = **pargv;
-			if (c == '\0') {
-				pargv++;
-			} else {
-				(*pargv)++;
-				goto loop_arg_is_opt;
-			}
-		}
-	}
-
-	if (spec_flgs & FREE_FIRST_ARGV_IS_OPT)
-		free(argv[1]);
-
 	/* check depending requires for given options */
 	for (on_off = complementary; on_off->opt_char; on_off++) {
 		if (on_off->requires && (flags & on_off->switch_on) &&

Modified: trunk/busybox/procps/top.c
===================================================================
--- trunk/busybox/procps/top.c	2008-08-19 23:02:23 UTC (rev 23110)
+++ trunk/busybox/procps/top.c	2008-08-20 00:12:22 UTC (rev 23111)
@@ -763,8 +763,7 @@
 
 	/* all args are options; -n NUM */
 	opt_complementary = "-:n+";
-	getopt32(argv, "d:n:b", &sinterval, &iterations);
-	if (option_mask32 & OPT_d) {
+	if (getopt32(argv, "d:n:b", &sinterval, &iterations) & OPT_d) {
 		/* Need to limit it to not overflow poll timeout */
 		interval = xatou16(sinterval); // -d
 	}