On Wed, Feb 16, 2011 at 09:18:57AM +0100, Svante Signell wrote:
> Did you get in contact with the upstream developers? How does your
> modified patch look like?
The RedHat folks have come up with the following:
Hi,
I'm sorry, there was a bug in the patch I've sent to you in previous
email. I've totally forgotten the for cycle because of poor code
readibility and it double frees in some cases :/. Sending proper patch now.
Please check it anyway, it's pretty easy to get lost there.
Regards,
Jan Kaluza
--
Paul Martin <[email protected]>
Index: logrotate.c
===================================================================
--- logrotate.c (revision 307)
+++ logrotate.c (working copy)
@@ -694,7 +694,6 @@
{
struct tm now = *localtime(&nowSecs);
char *oldName, *newName = NULL;
- char *tmp;
char *compext = "";
char *fileext = "";
int hasErrors = 0;
@@ -739,10 +738,6 @@
rotNames->baseName = strdup(ourBaseName(log->files[logNum]));
- oldName = alloca(PATH_MAX);
- newName = alloca(PATH_MAX);
- rotNames->disposeName = malloc(PATH_MAX);
-
if (log->extension &&
strncmp(&
(rotNames->
@@ -853,7 +848,7 @@
for (i = 0; i < globResult.gl_pathc && !hasErrors; i++) {
struct stat sbprev;
- snprintf(oldName, PATH_MAX, "%s", (globResult.gl_pathv)[i]);
+ asprintf(&oldName, "%s", (globResult.gl_pathv)[i]);
if (stat(oldName, &sbprev)) {
message(MESS_DEBUG,
"previous log %s does not exist\n",
@@ -861,20 +856,21 @@
} else {
hasErrors = compressLogFile(oldName, log, &sbprev);
}
+ free(oldName);
}
} else {
message(MESS_DEBUG,
"glob finding logs to compress failed\n");
/* fallback to old behaviour */
- snprintf(oldName, PATH_MAX, "%s/%s.%d%s", rotNames->dirName,
+ asprintf(&oldName, "%s/%s.%d%s", rotNames->dirName,
rotNames->baseName, logStart, fileext);
}
globfree(&globResult);
free(glob_pattern);
+ free(oldName);
} else {
struct stat sbprev;
-
- snprintf(oldName, PATH_MAX, "%s/%s.%d%s", rotNames->dirName,
+ asprintf(&oldName, "%s/%s.%d%s", rotNames->dirName,
rotNames->baseName, logStart, fileext);
if (stat(oldName, &sbprev)) {
message(MESS_DEBUG, "previous log %s does not exist\n",
@@ -882,6 +878,7 @@
} else {
hasErrors = compressLogFile(oldName, log, &sbprev);
}
+ free(oldName);
}
}
@@ -932,8 +929,10 @@
}
if (mail_out != -1) {
/* oldName is oldest Backup found (for unlink later) */
- snprintf(oldName, PATH_MAX, "%s", (globResult.gl_pathv)[mail_out]);
+ asprintf(&oldName, "%s", (globResult.gl_pathv)[mail_out]);
+ rotNames->disposeName = malloc(strlen(oldName)+1);
strcpy(rotNames->disposeName, oldName);
+ free(oldName);
} else {
free(rotNames->disposeName);
rotNames->disposeName = NULL;
@@ -952,7 +951,7 @@
if (log->rotateAge) {
struct stat fst_buf;
for (i = 1; i <= rotateCount + 1; i++) {
- snprintf(oldName, PATH_MAX, "%s/%s.%d%s%s", rotNames->dirName,
+ asprintf(&oldName, "%s/%s.%d%s%s", rotNames->dirName,
rotNames->baseName, i, fileext, compext);
if (!stat(oldName, &fst_buf)
&& (((nowSecs - fst_buf.st_mtime) / 60 / 60 / 24)
@@ -965,15 +964,16 @@
if (!hasErrors)
hasErrors = removeLogFile(mailFilename, log);
}
+ free(oldName);
}
}
- snprintf(oldName, PATH_MAX, "%s/%s.%d%s%s", rotNames->dirName,
+ asprintf(&oldName, "%s/%s.%d%s%s", rotNames->dirName,
rotNames->baseName, logStart + rotateCount, fileext,
compext);
- strcpy(newName, oldName);
+ newName = strdup(oldName);
- strcpy(rotNames->disposeName, oldName);
+ rotNames->disposeName = strdup(oldName);
sprintf(rotNames->firstRotated, "%s/%s.%d%s%s", rotNames->dirName,
rotNames->baseName, logStart, fileext,
@@ -1014,11 +1014,11 @@
}
}
#endif
+
for (i = rotateCount + logStart - 1; (i >= 0) && !hasErrors; i--) {
- tmp = newName;
- newName = oldName;
- oldName = tmp;
- snprintf(oldName, PATH_MAX, "%s/%s.%d%s%s", rotNames->dirName,
+ free(newName);
+ newName = oldName;
+ asprintf(&oldName, "%s/%s.%d%s%s", rotNames->dirName,
rotNames->baseName, i, fileext, compext);
message(MESS_DEBUG,
@@ -1035,24 +1035,28 @@
hasErrors = 1;
}
}
+
}
+ free(newName);
+ free(oldName);
} /* !LOG_FLAG_DATEEXT */
if (log->flags & LOG_FLAG_DATEEXT) {
- char *destFile = alloca(PATH_MAX);
+ char *destFile;
struct stat fst_buf;
if (asprintf(&(rotNames->finalName), "%s/%s%s%s", rotNames->dirName,
rotNames->baseName, dext_str, fileext) < 0) {
message(MESS_ERROR, "could not allocate finalName memory\n");
}
- snprintf(destFile, PATH_MAX, "%s%s", rotNames->finalName, compext);
+ asprintf(&destFile, "%s%s", rotNames->finalName, compext);
if (!stat(destFile, &fst_buf)) {
message(MESS_DEBUG,
"destination %s already exists, skipping rotation\n",
rotNames->firstRotated);
hasErrors = 1;
}
+ free(destFile);
} else {
/* note: the gzip extension is *not* used here! */
if (asprintf(&(rotNames->finalName), "%s/%s.%d%s", rotNames->dirName,
Index: config.c
===================================================================
--- config.c (revision 307)
+++ config.c (working copy)
@@ -222,7 +222,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])))
@@ -230,15 +230,16 @@
/* 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))
{
+ free(pattern);
message(MESS_DEBUG, "Ignoring %s, because of %s ending\n",
fname, tabooExts[i]);
return 0;
}
}
-
+ free(pattern);
/* All checks have been passed; return true */
return 1;
}
