On Mon, Feb 14, 2011 at 09:14:09AM +0100, Svante Signell wrote:
> diff -ur logrotate-3.7.8.orig//config.c ./logrotate-3.7.8/config.c
> --- logrotate-3.7.8.orig//config.c 2011-02-12 18:00:44.000000000 +0100
> +++ logrotate-3.7.8/config.c 2011-02-13 16:08:56.000000000 +0100
> @@ -160,7 +160,7 @@
> static int checkFile(const char *fname)
> {
> int i;
> - char pattern[PATH_MAX];
> + char *pattern;
>
> /* Check if fname is '.' or '..'; if so, return false */
> if (fname[0] == '.' && (!fname[1] || (fname[1] == '.' && !fname[2])))
> @@ -168,7 +168,7 @@
>
> /* Check if fname is ending in a taboo-extension; if so, return false */
> for (i = 0; i < tabooCount; i++) {
> - snprintf(pattern, sizeof(pattern), "*%s", tabooExts[i]);
> + asprintf(&pattern, "*%s", tabooExts[i]);
> if (!fnmatch(pattern, fname, 0))
> {
> message(MESS_DEBUG, "Ignoring %s, because of %s
> ending\n",
> @@ -176,7 +176,7 @@
> return 0;
> }
> }
> -
> + free(pattern);
> /* All checks have been passed; return true */
> return 1;
> }
You have a potential memory leak here. Only the "true" return does
free(pattern).
> + newName = alloca(strlen(oldName)+1);
> strcpy(newName, oldName);
>
> + rotNames->disposeName = malloc(strlen(oldName)+1);
> strcpy(rotNames->disposeName, oldName);
strdup(), perhaps?
> + newName = alloca(strlen(oldName)+1);
> newName = oldName;
> + oldName = alloca(strlen(tmp)+1);
> oldName = tmp;
strdup()?
I'd be *very* hesitant in dynamically allocating from the stack at
runtime. Just because you have a generous stack allocation on i386
doesn't mean that there is one on other architectures. If you overrun
the stack with alloca(), your program will hopefully segfault.
Thanks for your contribution. I'll use this as the basis of a patch.
--
Paul Martin <[email protected]>
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
