RPM Community Forums

Mailing List Message of <popt-devel>

[PATCH] get rid of ancient alloca(3) use

From: Ralf S. Engelschall <rse@rpm5.org>
Date: Thu 14 Jun 2007 - 14:11:27 CEST
Message-ID: <20070614121127.GA67501@engelschall.com>
I have one subtle issue on my RPM/POPT todo list since a longer
time: POPT's use of the ancient 32V AT&T UNIX alloca(3), a machine-,
compiler-, and system-dependent and especially non-POSIX (and hence
less portable) function whose use is even discouraged on lots of Unix
platforms.

POPT even uses it in a IMHO unecessary way to allocate temporary
structures on the stack:

     poptDone done = alloca(sizeof(*done));

This can be just replaced by the following and let the compiler do
mostly all the work already under compile-time:

     struct poptDone_s done_buf;
     poptDone done = &done_buf;

The following patch completely kicks out all alloca(3) usages from POPT.
Either through the above replacement, or by not requiring a temporary
buffer at all (by passing length argument to internal findOption()
function) or in the worst cases by replacing with the POSIX malloc(3)
and calloc(3) functions.

I carefully performed the replacements, reviewed them individually and
the POPT "make check" still works, of course.

Any objections?
                                       Ralf S. Engelschall
                                       rse@engelschall.com
                                       www.engelschall.com

Index: configure.ac
===================================================================
RCS file: /v/rpm/cvs/popt/configure.ac,v
retrieving revision 1.15
diff -u -d -u -d -r1.15 configure.ac
--- configure.ac	14 Jun 2007 06:42:07 -0000	1.15
+++ configure.ac	14 Jun 2007 11:38:45 -0000
@@ -67,7 +67,7 @@
 fi
 AC_SUBST(MARK64)

-AC_CHECK_HEADERS(alloca.h float.h libintl.h mcheck.h unistd.h)
+AC_CHECK_HEADERS(float.h libintl.h mcheck.h unistd.h)

 # For some systems we know that we have ld_version scripts.
 # Use it then as default.
Index: findme.c
===================================================================
RCS file: /v/rpm/cvs/popt/findme.c,v
retrieving revision 1.16
diff -u -d -u -d -r1.16 findme.c
--- findme.c	14 Jun 2007 08:22:43 -0000	1.16
+++ findme.c	14 Jun 2007 11:38:45 -0000
@@ -12,9 +12,9 @@
 const char * POPT_findProgramPath(const char * argv0)
 {
     char * path = getenv("PATH");
-    char * pathbuf;
+    char * pathbuf = NULL;
     char * start, * chptr;
-    char * buf;
+    char * buf = NULL;

     if (argv0 == NULL) return NULL;	/* XXX can't happen */
     /* If there is a / in the argv[0], it has to be an absolute path */
@@ -23,9 +23,10 @@

     if (path == NULL) return NULL;

-    start = pathbuf = alloca(strlen(path) + 1);
+    start = pathbuf = malloc(strlen(path) + 1);
+    if (pathbuf == NULL) goto exit;
     buf = malloc(strlen(path) + strlen(argv0) + sizeof("/"));
-    if (buf == NULL) return NULL;	/* XXX can't happen */
+    if (buf == NULL) goto exit;
     strcpy(pathbuf, path);

     chptr = NULL;
@@ -35,8 +36,10 @@
 	    *chptr = '\0';
 	sprintf(buf, "%s/%s", start, argv0);

-	if (!access(buf, X_OK))
+	if (!access(buf, X_OK)) {
+	    free(pathbuf);
 	    return buf;
+	}

 	if (chptr)
 	    start = chptr + 1;
@@ -45,7 +48,11 @@
     } while (start && *start);
 /*@=branchstate@*/

-    free(buf);
+exit:
+    if (pathbuf)
+        free(pathbuf);
+    if (buf)
+        free(buf);

     return NULL;
 }
Index: popt.c
===================================================================
RCS file: /v/rpm/cvs/popt/popt.c,v
retrieving revision 1.100
diff -u -d -u -d -r1.100 popt.c
--- popt.c	14 Jun 2007 08:22:43 -0000	1.100
+++ popt.c	14 Jun 2007 11:38:45 -0000
@@ -394,8 +394,9 @@
     if (argv == NULL) return POPT_ERROR_MALLOC;

     if (!strchr(item->argv[0], '/') && con->execPath != NULL) {
-	char *s = alloca(strlen(con->execPath) + strlen(item->argv[0]) + sizeof("/"));
-	sprintf(s, "%s/%s", con->execPath, item->argv[0]);
+	char *s = malloc(strlen(con->execPath) + strlen(item->argv[0]) + sizeof("/"));
+	if (s)
+	    sprintf(s, "%s/%s", con->execPath, item->argv[-1]);
 	argv[argc] = s;
     } else
 	argv[argc] = POPT_findProgramPath(item->argv[0]);
@@ -448,11 +449,6 @@
 #endif
 #endif

-    if (argv[0] == NULL) {
-	ec = POPT_ERROR_NOARG;
-	goto exit;
-    }
-
 #ifdef	MYDEBUG
 if (_popt_debug)
     {	const char ** avp;
@@ -466,14 +462,18 @@
     rc = execvp(argv[0], (char *const *)argv);

 exit:
-    if (argv) free(argv);
+    if (argv) {
+        if (argv[0])
+            free((void *)argv[0]);
+        free(argv);
+    }
     return ec;
 }
 /*@=bounds =boundswrite @*/

 /*@-boundswrite@*/
 /*@observer@*/ /*@null@*/ static const struct poptOption *
-findOption(const struct poptOption * opt, /*@null@*/ const char * longName,
+findOption(const struct poptOption * opt, /*@null@*/ const char * longName, int longNameLen,
 		char shortName,
 		/*@null@*/ /*@out@*/ poptCallbackType * callback,
 		/*@null@*/ /*@out@*/ const void ** callbackData,
@@ -498,7 +498,7 @@
 /*@=branchstate@*/
 	    /* Recurse on included sub-tables. */
 	    if (arg == NULL) continue;	/* XXX program error */
-	    opt2 = findOption(arg, longName, shortName, callback,
+	    opt2 = findOption(arg, longName, longNameLen, shortName, callback,
 			      callbackData, singleDash);
 	    if (opt2 == NULL) continue;
 	    /* Sub-table data will be inheirited if no data yet. */
@@ -512,7 +512,7 @@
 	    cb = opt;
 	} else if (longName != NULL && opt->longName != NULL &&
 		   (!singleDash || (opt->argInfo & POPT_ARGFLAG_ONEDASH)) &&
-		   !strcmp(longName, opt->longName))
+		   (!strncmp(longName, opt->longName, longNameLen) && strlen(opt->longName) == longNameLen))
 	{
 	    break;
 	} else if (shortName && shortName == opt->shortName) {
@@ -764,7 +764,8 @@

 	/* Process next long option */
 	if (!con->os->nextCharArg) {
-	    char * localOptString, * optString;
+	    char * optString;
+            int optStringLen;
 	    int thisopt;

 /*@-sizeoftype@*/
@@ -795,8 +796,7 @@
 	    }

 	    /* Make a copy we can hack at */
-	    localOptString = optString =
-		strcpy(alloca(strlen(origOptString) + 1), origOptString);
+	    optString = origOptString;

 	    if (optString[0] == '\0')
 		return POPT_ERROR_BADOPT;
@@ -824,13 +824,11 @@
 		/* Check for "--long=arg" option. */
 		for (oe = optString; *oe && *oe != '='; oe++)
 		    {};
-		if (*oe == '=') {
-		    *oe++ = '\0';
-		    /* XXX longArg is mapped back to persistent storage. */
-		    longArg = origOptString + (oe - localOptString);
-		}
+		optStringLen = oe - optString;
+		if (*oe == '=')
+		    longArg = oe + 1;

-		opt = findOption(con->options, optString, '\0', &cb, &cbData,
+		opt = findOption(con->options, optString, optStringLen, '\0', &cb, &cbData,
 				 singleDash);
 		if (!opt && !singleDash)
 		    return POPT_ERROR_BADOPT;
@@ -867,7 +865,7 @@
 		continue;
 	    }

-	    opt = findOption(con->options, NULL, *origOptString, &cb,
+	    opt = findOption(con->options, NULL, 0, *origOptString, &cb,
 			     &cbData, 0);
 	    if (!opt)
 		return POPT_ERROR_BADOPT;
@@ -1140,7 +1138,8 @@
 int poptAddAlias(poptContext con, struct poptAlias alias,
 		/*@unused@*/ int flags)
 {
-    poptItem item = alloca(sizeof(*item));
+    struct poptItem_s item_buf;
+    poptItem item = &item_buf;
     memset(item, 0, sizeof(*item));
     item->option.longName = alias.longName;
     item->option.shortName = alias.shortName;
Index: poptconfig.c
===================================================================
RCS file: /v/rpm/cvs/popt/poptconfig.c,v
retrieving revision 1.27
diff -u -d -u -d -r1.27 poptconfig.c
--- poptconfig.c	25 May 2007 17:36:23 -0000	1.27
+++ poptconfig.c	14 Jun 2007 11:38:45 -0000
@@ -17,7 +17,8 @@
     size_t nameLength;
     const char * entryType;
     const char * opt;
-    poptItem item = alloca(sizeof(*item));
+    struct poptItem_s item_buf;
+    poptItem item = &item_buf;
     int i, j;

     if (con->appName == NULL)
@@ -95,8 +96,8 @@

 int poptReadConfigFile(poptContext con, const char * fn)
 {
-    const char * file, * chptr, * end;
-    char * buf;
+    char * file = NULL, * chptr, * end;
+    char * buf = NULL;
 /*@dependent@*/ char * dst;
     int fd, rc;
     off_t fileLength;
@@ -113,18 +114,20 @@
 	return POPT_ERROR_ERRNO;
     }

-    file = alloca(fileLength + 1);
+    file = malloc(fileLength + 1);
     if (read(fd, (char *)file, fileLength) != fileLength) {
 	rc = errno;
 	(void) close(fd);
 	errno = rc;
 	return POPT_ERROR_ERRNO;
     }
-    if (close(fd) == -1)
+    if (close(fd) == -1) {
+	free(file);
 	return POPT_ERROR_ERRNO;
+    }

 /*@-boundswrite@*/
-    dst = buf = alloca(fileLength + 1);
+    dst = buf = malloc(fileLength + 1);

     chptr = file;
     end = (file + fileLength);
@@ -157,6 +160,9 @@
 /*@=infloops@*/
 /*@=boundswrite@*/

+    free(file);
+    free(buf);
+
     return 0;
 }

@@ -171,10 +177,11 @@
     if (rc) return rc;

     if ((home = getenv("HOME"))) {
-	fn = alloca(strlen(home) + 20);
+	fn = malloc(strlen(home) + 20);
 	strcpy(fn, home);
 	strcat(fn, "/.popt");
 	rc = poptReadConfigFile(con, fn);
+	free(fn);
 	if (rc) return rc;
     }

Index: popthelp.c
===================================================================
RCS file: /v/rpm/cvs/popt/popthelp.c,v
retrieving revision 1.56
diff -u -d -u -d -r1.56 popthelp.c
--- popthelp.c	14 Jun 2007 08:08:30 -0000	1.56
+++ popthelp.c	14 Jun 2007 11:38:45 -0000
@@ -767,11 +767,14 @@
 	/*@modifies *str, *fp, fileSystem @*/
 	/*@requires maxRead(str) >= 0 @*/
 {
-    /* bufsize larger then the ascii set, lazy alloca on top level call. */
+    /* bufsize larger then the ascii set, lazy allocation on top level call. */
     size_t nb = (size_t)300;
-    char * s = (str != NULL ? str : memset(alloca(nb), 0, nb));
+    char * s = (str != NULL ? str : calloc(1, nb));
     size_t len = (size_t)0;

+    if (s == NULL)
+	return 0;
+
 /*@-boundswrite@*/
     if (opt != NULL)
     for (; (opt->longName || opt->shortName || opt->arg); opt++) {
@@ -788,19 +791,23 @@
 	fprintf(fp, " [-%s]", s);
 	len = strlen(s) + sizeof(" [-]")-1;
     }
+    if (s != str)
+	free(s);
     return len;
 }

 void poptPrintUsage(poptContext con, FILE * fp, /*@unused@*/ int flags)
 {
-    poptDone done = memset(alloca(sizeof(*done)), 0, sizeof(*done));
+    struct poptDone_s done_buf;
+    poptDone done = &done_buf;
     size_t cursor;

+    memset(done, 0, sizeof(*done));
     done->nopts = 0;
     done->maxopts = 64;
     cursor = done->maxopts * sizeof(*done->opts);
 /*@-boundswrite@*/
-    done->opts = memset(alloca(cursor), 0, cursor);
+    done->opts = calloc(1, cursor);
     /*@-keeptrans@*/
     done->opts[done->nopts++] = (const void *) con->options;
     /*@=keeptrans@*/
@@ -819,6 +826,7 @@
     }

     fprintf(fp, "\n");
+    free(done->opts);
 }

 void poptSetOtherOptionHelp(poptContext con, const char * text)
Index: poptparse.c
===================================================================
RCS file: /v/rpm/cvs/popt/poptparse.c,v
retrieving revision 1.22
diff -u -d -u -d -r1.22 poptparse.c
--- poptparse.c	14 Jun 2007 08:04:38 -0000	1.22
+++ poptparse.c	14 Jun 2007 11:38:45 -0000
@@ -62,10 +62,12 @@
     const char ** argv = malloc(sizeof(*argv) * argvAlloced);
     int argc = 0;
     size_t buflen = strlen(s) + 1;
-    char * buf = memset(alloca(buflen), 0, buflen);
+    char * buf, * bufOrig = NULL;
     int rc = POPT_ERROR_MALLOC;

     if (argv == NULL) return rc;
+    buf = bufOrig = calloc(1, buflen);
+    if (buf == NULL) return rc;
     argv[argc] = buf;

     for (src = s; *src != '\0'; src++) {
@@ -116,6 +118,7 @@
     rc = poptDupArgv(argc, argv, argcPtr, argvPtr);

 exit:
+    if (bufOrig) free(bufOrig);
     if (argv) free(argv);
     return rc;
 }
Index: system.h
===================================================================
RCS file: /v/rpm/cvs/popt/system.h,v
retrieving revision 1.9
diff -u -d -u -d -r1.9 system.h
--- system.h	14 Jun 2007 08:16:01 -0000	1.9
+++ system.h	14 Jun 2007 11:38:45 -0000
@@ -39,32 +39,6 @@
 #include <libc.h>
 #endif

-#if defined(__LCLINT__)
-/*@-declundef -incondefs @*/ /* LCL: missing annotation */
-/*@only@*/ /*@out@*/
-void * alloca (size_t __size)
-	/*@ensures MaxSet(result) == (__size - 1) @*/
-	/*@*/;
-/*@=declundef =incondefs @*/
-#endif
-
-/* AIX requires this to be the first thing in the file.  */
-#ifndef __GNUC__
-# ifdef HAVE_ALLOCA_H
-#  include <alloca.h>
-# else
-#  ifdef _AIX
-#pragma alloca
-#  else
-#   ifndef alloca /* predefined by HP cc +Olibcalls */
-char *alloca ();
-#   endif
-#  endif
-# endif
-#elif defined(__GNUC__) && defined(__STRICT_ANSI__)
-#define alloca __builtin_alloca
-#endif
-
 /*@-redecl -redef@*/
 /*@mayexit@*/ /*@only@*/ /*@unused@*/
 char * xstrdup (const char *str)
Received on Thu Jun 14 14:12:39 2007
Driven by Jeff Johnson and the RPM project team.
Hosted by OpenPKG and Ralf S. Engelschall.
Powered by FreeBSD and OpenPKG.