On Sun, Mar 09, 2008 at 08:27:25PM -0400, Jeff Johnson wrote:
> Using xmalloc just opens up a can-of-worms while lusers fuss about
> non-gcc compiler extension portability.
Aha, I had failed to notice that the "? :" bit was a gcc extension.
What I don't like is that normal users get some memory functions that
are fatal if they use gcc, while all others get non-fatal functions.
Perhaps the default should be for all normal compiles to get the non-
fatal functions, and then anyone that wanted to use the current gcc
defines could set something extra (such as -DFATAL_MEM).
One other reason I care about this is that I think the various bits of
code that are doing a strdup()-like action should be using a strdup()-
like call, not each one having their own set of strlen/malloc/strcpy
calls. It's clearer and a bit safer.
I'm attaching a patch that adds the extra FATAL_MEM requirement, and
then uses xstrdup() in a few places (which would mean that strdup() is
being used for normal popt users).
I was also working on changing some code to use your newly-added
stpcpy() function, so I included that work too. Some of the changes
are optimizations to avoid a strlen() call (when we can easily compute
the length via subtraction) and a fix for a potential memory leak if
realloc() returns NULL.
..wayne..
--- system.h 9 Mar 2008 20:24:45 -0000 1.14
+++ system.h 10 Mar 2008 01:40:23 -0000
@@ -78,7 +78,7 @@ static inline char * stpcpy (char *dest,
#endif
/* Memory allocation via macro defs to get meaningful locations from mtrace()
*/
-#if defined(HAVE_MCHECK_H) && defined(__GNUC__)
+#if defined(FATAL_MEM) && defined(HAVE_MCHECK_H) && defined(__GNUC__)
#define vmefail() (fprintf(stderr, "virtual memory
exhausted.\n"), exit(EXIT_FAILURE), NULL)
#define xmalloc(_size) (malloc(_size) ? : vmefail())
#define xcalloc(_nmemb, _size) (calloc((_nmemb), (_size)) ? :
vmefail())
--- findme.c 9 Mar 2008 20:24:45 -0000 1.19
+++ findme.c 10 Mar 2008 01:40:21 -0000
@@ -23,11 +23,10 @@ const char * POPT_findProgramPath(const
if (path == NULL) return NULL;
- start = pathbuf = malloc(strlen(path) + 1);
+ start = pathbuf = xstrdup(path);
if (pathbuf == NULL) goto exit;
buf = malloc(strlen(path) + strlen(argv0) + sizeof("/"));
if (buf == NULL) goto exit;
- strcpy(pathbuf, path);
chptr = NULL;
do {
--- popt.c 9 Mar 2008 23:10:05 -0000 1.120
+++ popt.c 10 Mar 2008 01:40:22 -0000
@@ -196,10 +196,8 @@ poptContext poptGetContext(const char *
if (getenv("POSIXLY_CORRECT") || getenv("POSIX_ME_HARDER"))
con->flags |= POPT_CONTEXT_POSIXMEHARDER;
- if (name) {
- char * t = malloc(strlen(name) + 1);
- if (t) con->appName = strcpy(t, name);
- }
+ if (name)
+ con->appName = xstrdup(name);
invokeCallbacksPRE(con, con->options);
@@ -593,7 +591,7 @@ expandNextArg(/[EMAIL PROTECTED]@*/ poptContext
/[EMAIL PROTECTED] con @*/
{
const char * a = NULL;
- size_t alen;
+ size_t alen, pos;
char *t, *te;
size_t tn = strlen(s) + 1;
char c;
@@ -619,10 +617,9 @@ expandNextArg(/[EMAIL PROTECTED]@*/ poptContext
alen = strlen(a);
tn += alen;
- *te = '\0';
+ pos = te - t;
t = realloc(t, tn);
- te = t + strlen(t);
- strncpy(te, a, alen); te += alen;
+ te = stpcpy(t + pos, a);
continue;
/[EMAIL PROTECTED]@*/ /[EMAIL PROTECTED]@*/ break;
default:
@@ -630,8 +627,13 @@ expandNextArg(/[EMAIL PROTECTED]@*/ poptContext
}
*te++ = c;
}
- *te = '\0';
- t = realloc(t, strlen(t) + 1); /* XXX memory leak, hard to plug */
+ *te++ = '\0';
+ if (t + tn > te) {
+ char *ot = t;
+ t = realloc(t, te - t);
+ if (!t)
+ free(ot);
+ }
return t;
}
--- poptconfig.c 9 Mar 2008 20:24:45 -0000 1.37
+++ poptconfig.c 10 Mar 2008 01:40:22 -0000
@@ -224,8 +224,7 @@ int poptReadDefaultConfig(poptContext co
if ((home = getenv("HOME"))) {
fn = malloc(strlen(home) + 20);
if (fn != NULL) {
- strcpy(fn, home);
- strcat(fn, "/.popt");
+ (void)stpcpy(stpcpy(fn, home), "/.popt");
rc = poptReadConfigFile(con, fn);
free(fn);
} else
--- popthelp.c 8 Mar 2008 17:26:30 -0000 1.85
+++ popthelp.c 10 Mar 2008 01:40:23 -0000
@@ -237,7 +237,7 @@ singleOptionDefaultValue(size_t lineLeng
if (le == NULL) return NULL; /* XXX can't happen */
*le = '\0';
*le++ = '(';
- strcpy(le, defstr); le += strlen(le);
+ le = stpcpy(le, defstr);
*le++ = ':';
*le++ = ' ';
if (opt->arg) { /* XXX programmer error */
@@ -269,7 +269,7 @@ singleOptionDefaultValue(size_t lineLeng
case POPT_ARG_STRING:
{ const char * s = arg.argv[0];
if (s == NULL) {
- strcpy(le, "null"); le += strlen(le);
+ le = stpcpy(le, "null");
} else {
size_t slen = 4*lineLength - (le - l) - sizeof("\"...\")");
*le++ = '"';
@@ -332,18 +332,20 @@ static void singleOptionHelp(FILE * fp,
#define prtlong (opt->longName != NULL) /* XXX splint needs a clue */
if (!(prtshort || prtlong))
goto out;
- if (prtshort && prtlong)
- sprintf(left, "-%c, %s%s", opt->shortName,
- (F_ISSET(opt, ONEDASH) ? "-" : "--"),
- opt->longName);
- else if (prtshort)
- sprintf(left, "-%c", opt->shortName);
- else if (prtlong)
+ if (prtshort && prtlong) {
+ char *dash = F_ISSET(opt, ONEDASH) ? "-" : "--";
+ left[0] = '-';
+ left[1] = opt->shortName;
+ (void)stpcpy(stpcpy(stpcpy(left+2, ", "), dash), opt->longName);
+ } else if (prtshort) {
+ left[0] = '-';
+ left[1] = opt->shortName;
+ } else if (prtlong) {
+ char *dash = poptArgType(opt) == POPT_ARG_MAINCALL ? ""
+ : (F_ISSET(opt, ONEDASH) ? "-" : "--");
/* XXX --long always padded for alignment with/without "-X, ". */
- sprintf(left, " %s%s",
- (poptArgType(opt) == POPT_ARG_MAINCALL ? "" :
- (F_ISSET(opt, ONEDASH) ? "-" : "--")),
- opt->longName);
+ (void)stpcpy(stpcpy(stpcpy(left, " "), dash), opt->longName);
+ }
#undef prtlong
if (argDescrip) {
@@ -361,9 +363,8 @@ static void singleOptionHelp(FILE * fp,
if (t) {
char * te = t;
*te = '\0';
- if (help) {
- strcpy(te, help); te += strlen(te);
- }
+ if (help)
+ te = stpcpy(te, help);
*te++ = ' ';
strcpy(te, defs);
defs = _free(defs);
