Bug 166510
Summary: | Logrotate should rotate logs even if there are some errors on the configuration | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Filipe Brandenburger <filbranden> |
Component: | logrotate | Assignee: | Peter Vrabec <pvrabec> |
Status: | CLOSED RAWHIDE | QA Contact: | |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | tao |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-23 12:18:29 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 218388 | ||
Attachments: |
Description
Filipe Brandenburger
2005-08-22 17:55:42 UTC
Created attachment 134926 [details]
A patch fixing few issues with config file parsing
The patch fixes this bug, #185538 and #182062. It's very simple - if logrotate
finds a duplicate filename (or corrup config file), it skips it and continues
with the next one.
Thnx. for patch, but it doesn't fix all cases. If there is a bug in local scope of configuration file, log file won't be skipped. example: $ cat rot/foo2 create missingok /tmp/foo2 { rotate -3 } $sudo logrotate-3.7.4/logrotate -vf rotFile ... reading config info for /tmp/foo2 error: foo2:4 bad rotation count '-3' error: found error in foo2, skipping reading config file foo3 .... rotating pattern: /tmp/foo2 forced from command line (-3 rotations) empty log files are rotated, old logs are removed considering log /tmp/foo2 log needs rotating rotating log /tmp/foo2, log->rotateCount is -3 log /tmp/foo2.-2 doesn't exist -- won't try to dispose of it renaming /tmp/foo2 to /tmp/foo2.1 creating new log mode = 0664 uid = 500 gid = 500 ... Created attachment 135366 [details]
A patch fixing few issues with config file parsing
New version of the patch.
Comment on attachment 135366 [details] A patch fixing few issues with config file parsing >--- logrotate-3.7.4/config.c.old 2006-07-24 14:08:04.000000000 +0200 >+++ logrotate-3.7.4/config.c 2006-09-01 11:56:52.000000000 +0200 >@@ -267,10 +267,8 @@ > > if (readConfigFile(namelist[i], defConfig, logsPtr, > numLogsPtr)) { >- fchdir(here); >- close(here); >- free_2d_array(namelist, files_count); >- return 1; >+ message(MESS_ERROR, "found error in %s, skipping\n", namelist[i]); >+ continue; > } > } > >@@ -364,6 +362,7 @@ > glob_t globResult; > const char **argv; > int argc, argNum; >+ int logerror = 0; > > /* FIXME: createOwner and createGroup probably shouldn't be fixed > length arrays -- of course, if we aren't run setuid it doesn't >@@ -417,6 +416,34 @@ > > start = buf; > while (*start) { >+ if (logerror) { >+ while (*start != '}') { >+ if (*start == 0) { >+ message(MESS_ERROR, "%s:%d } expected \n", configFile, lineNum); >+ return 1; >+ } >+ else if (*start == '\n') { >+ lineNum++; >+ } >+ start++; >+ } >+ >+ if (newlog->files) >+ free(newlog->files); >+ if (newlog->pre && newlog->pre != defConfig->pre) >+ free(newlog->pre); >+ if (newlog->post && newlog->post != defConfig->post) >+ free(newlog->post); >+ if (newlog->first && newlog->first != defConfig->first) >+ free(newlog->first); >+ if (newlog->last && newlog->last != defConfig->last) >+ free(newlog->last); >+ (*numLogsPtr)--; >+ *logsPtr = realloc(*logsPtr, sizeof(**logsPtr) * *numLogsPtr); >+ newlog = defConfig; >+ >+ logerror = 0; >+ } > while (isblank(*start) && (*start)) > start++; > if (*start == '#') { >@@ -549,7 +576,12 @@ > if (rc == 4) { > message(MESS_ERROR, "%s:%d extra arguments for " > "create\n", configFile, lineNum); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > if (rc > 0) >@@ -560,7 +592,12 @@ > if (!pw) { > message(MESS_ERROR, "%s:%d unknown user '%s'\n", > configFile, lineNum, createOwner); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > newlog->createUid = pw->pw_uid; > endpwent(); >@@ -570,7 +607,12 @@ > if (!group) { > message(MESS_ERROR, "%s:%d unknown group '%s'\n", > configFile, lineNum, createGroup); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > newlog->createGid = group->gr_gid; > endgrent(); >@@ -605,7 +647,12 @@ > } else if (!isdigit(start[length])) { > message(MESS_ERROR, "%s:%d unknown unit '%c'\n", > configFile, lineNum, start[length]); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } else { > multiplier = 1; > } >@@ -614,7 +661,12 @@ > if (*chptr) { > message(MESS_ERROR, "%s:%d bad size '%s'\n", > configFile, lineNum, start); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > if (!strcmp(opt, "size")) { >@@ -676,7 +728,12 @@ > message(MESS_ERROR, > "%s:%d bad rotation count '%s'\n", > configFile, lineNum, start); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > *endtag = oldchar, start = endtag; > } >@@ -692,7 +749,12 @@ > if (*chptr || newlog->logStart < 0) { > message(MESS_ERROR, "%s:%d bad start count '%s'\n", > configFile, lineNum, start); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > *endtag = oldchar, start = endtag; > } >@@ -708,7 +770,12 @@ > if (*chptr || newlog->rotateAge < 0) { > message(MESS_ERROR, "%s:%d bad maximum age '%s'\n", > configFile, lineNum, start); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > *endtag = oldchar, start = endtag; > } >@@ -720,7 +787,12 @@ > *endtag = oldchar, start = endtag; > if (!(newlog->logAddress = readAddress(configFile, lineNum, > "mail", &start))) { >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > } else if (!strcmp(start, "nomail")) { > /* hmmm, we could lose memory here, but not much */ >@@ -773,7 +845,12 @@ > "%s:%d tabooext may not appear inside " > "of log file definition\n", configFile, > lineNum); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > *endtag = oldchar, start = endtag; >@@ -819,7 +896,12 @@ > "%s:%d include may not appear inside " > "of log file definition\n", configFile, > lineNum); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > *endtag = oldchar, start = endtag; >@@ -831,7 +913,12 @@ > > if (readConfigPath(start, defConfig, logsPtr, > numLogsPtr)) >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > > *endtag = oldchar, start = endtag; > } >@@ -839,7 +926,12 @@ > *endtag = oldchar, start = endtag; > if (!(newlog->oldDir = readPath(configFile, lineNum, > "olddir", &start))) { >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > #if 0 > if (stat(newlog->oldDir, &sb)) { >@@ -881,7 +973,12 @@ > if (! > (newlog->compress_prog = > readPath(configFile, lineNum, "compress", &start))) { >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > if (access(newlog->compress_prog, X_OK)) { >@@ -889,7 +986,12 @@ > "%s:%d compression program %s is not an executable file\n", > configFile, lineNum, newlog->compress_prog); > free(newlog->compress_prog); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > message(MESS_DEBUG, "compress_prog is now %s\n", >@@ -901,7 +1003,12 @@ > (newlog->uncompress_prog = > readPath(configFile, lineNum, "uncompress", > &start))) { >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > if (access(newlog->uncompress_prog, X_OK)) { >@@ -909,7 +1016,12 @@ > "%s:%d uncompression program %s is not an executable file\n", > configFile, lineNum, newlog->uncompress_prog); > free(newlog->uncompress_prog); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > message(MESS_DEBUG, "uncompress_prog is now %s\n", >@@ -923,7 +1035,12 @@ > (options = > readPath(configFile, lineNum, "compressoptions", > &start))) { >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > if (poptParseArgvString(options, >@@ -933,7 +1050,12 @@ > "%s:%d invalid compression options\n", > configFile, lineNum); > free(options); >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > message(MESS_DEBUG, "compress_options is now %s\n", >@@ -945,7 +1067,12 @@ > (newlog->compress_ext = > readPath(configFile, lineNum, "compress-ext", > &start))) { >- return 1; >+ if (newlog != defConfig) { >+ logerror = 1; >+ continue; >+ } else { >+ return 1; >+ } > } > > message(MESS_DEBUG, "compress_ext is now %s\n", >@@ -973,7 +1100,8 @@ > if (newlog != defConfig) { > message(MESS_ERROR, "%s:%d unexpected log filename\n", > configFile, lineNum); >- return 1; >+ logerror = 1; >+ continue; > } > > /* If no compression options were found in config file, >@@ -1042,11 +1170,10 @@ > if (!strcmp((*logsPtr)[j].files[k], > globResult.gl_pathv[i])) { > message(MESS_ERROR, >- "%s:%d duplicate log entry for %s\n", >+ "%s:%d duplicate log entry for %s, skipping\n", > configFile, lineNum, > globResult.gl_pathv[i]); >- globfree(&globResult); >- return 1; >+ continue; > } > } > } >@@ -1067,7 +1194,7 @@ > start = endtag + 1; > } else if (*start == '}') { > if (newlog == defConfig) { >- message(MESS_ERROR, "%s:%d unxpected }\n", configFile, >+ message(MESS_ERROR, "%s:%d unexpected }\n", configFile, > lineNum); > return 1; > } Created attachment 135370 [details]
A patch fixing few issues with config file parsing
Sorry about the above post (my bugzilla skill is not good enough).
Created attachment 135492 [details]
A new version of the patch
The patch fixes these issues:
a) When a config file is corrupt, logrotate skips it and rolls back all
changes.
b) When a filename in log config is duplicate, logrotate skips it.
c) When a log config is corrupt, logrotate skips it.
Created attachment 135558 [details]
New version of the patch
New fix - now when logrotate finds duplicate filename, it skips the whole log
config.
Created attachment 135738 [details]
another fix candidate
This is adjusted grubber's patch, it was necessary to fix memory leak .
Created attachment 135775 [details]
Another new version of the patch
More fixes, including Peter's changes.
Created attachment 136992 [details]
New version of the patch
Another version of the patch, now without memory leaks.
thnx. to Jan Cholasta fixed in: FC6(testing) logrotate-3.7.4-10.fc6, rawhide logrotate-3.7.4-11.fc7. |