Bug#277652: Ubuntu patch
Hi Paul, Paul Martin [2008-10-08 16:17 +0100]: > My response is to push the check to the end of the block. We're still going > to get people saying "why am I getting spammed by cron?" due to the error > messages. Indeed, that makes even more sense. Thanks for looking into it! > --- logrotate-3.7.1.orig/config.c 2008-10-08 15:57:39.318679248 +0100 > +++ logrotate-3.7.1/config.c 2008-10-08 16:09:33.203414313 +0100 > @@ -968,7 +970,8 @@ > > message(MESS_ERROR, "%s:%d glob failed for %s\n", > configFile, lineNum, argv[argNum]); > - return 1; > + glob_failed = 1; > + continue; Since you write the message in the hunk below, I think you need to drop it here, otherwise you'd write it twice without missingok and still once with missingok. Otherwise the idea is much better than my hack, indeed. Thanks! Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature
Bug#277652: Ubuntu patch
severity 277652 serious
tags 277652 patch pending
thanks
On Wed, Oct 08, 2008 at 02:10:17PM +0200, Martin Pitt wrote:
> in Ubuntu we consider this a release critical bug, because a single
> glob failure in any configuration file can break the entire logrotate
> system. Thus we applied a quick hack (thanks to Jean-Baptiste
> Lallement) which works around the problem. syslog will stil get the
> glob errors, but at least the following logrotate.d files will be
> processed. This kind of breaks "nomissingok", but I don't think that
> it was ever intended to break the *entire* log rotation, it should
> just stop the current config file.
My response is to push the check to the end of the block. We're still going
to get people saying "why am I getting spammed by cron?" due to the error
messages.
How does this look? (I'm going to be giving it some testing before
uploading.)
--- logrotate-3.7.1.orig/config.c 2008-10-08 15:57:39.318679248 +0100
+++ logrotate-3.7.1/config.c2008-10-08 16:09:33.203414313 +0100
@@ -370,6 +370,7 @@
int createMode;
struct stat sb, sb2;
glob_t globResult;
+int glob_failed;
const char ** argv;
int argc, argNum;
@@ -422,6 +423,7 @@
message(MESS_DEBUG, "reading config file %s\n", configFile);
+glob_failed = 0;
start = buf;
while (*start) {
while (isblank(*start) && (*start)) start++;
@@ -968,7 +970,8 @@
message(MESS_ERROR, "%s:%d glob failed for %s\n",
configFile, lineNum, argv[argNum]);
- return 1;
+ glob_failed = 1;
+ continue;
}
newlog->files = realloc(newlog->files, sizeof(*newlog->files) *
@@ -1011,6 +1014,12 @@
message(MESS_ERROR, "%s:%d unxpected }\n", configFile, lineNum);
return 1;
}
+
+ if (glob_failed && !(newlog->flags & LOG_FLAG_MISSINGOK)) {
+ message(MESS_ERROR, "%s:%d glob failure without missingok\n",
+ configFile, lineNum);
+ return 1;
+ }
if (newlog->oldDir) {
for (i = 0; i < newlog->numFiles; i++) {
@@ -1046,6 +1055,7 @@
}
newlog = defConfig;
+ glob_failed = 0;
start++;
while (isblank(*start)) start++;
--
Paul Martin <[EMAIL PROTECTED]>
signature.asc
Description: Digital signature
Bug#277652: Ubuntu patch
Hi Paul, in Ubuntu we consider this a release critical bug, because a single glob failure in any configuration file can break the entire logrotate system. Thus we applied a quick hack (thanks to Jean-Baptiste Lallement) which works around the problem. syslog will stil get the glob errors, but at least the following logrotate.d files will be processed. This kind of breaks "nomissingok", but I don't think that it was ever intended to break the *entire* log rotation, it should just stop the current config file. I think it's appropriate for Lenny as well, but that's your call, of course. Thank you, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) diff -u logrotate-3.7.1/debian/changelog logrotate-3.7.1/debian/changelog --- logrotate-3.7.1/debian/changelog +++ logrotate-3.7.1/debian/changelog @@ -1,3 +1,14 @@ +logrotate (3.7.1-3ubuntu2) intrepid; urgency=low + + * Add debian/patches/glob-failure.patch: Prevent logrotate from failing +completely if one glob in any included file fails. This more or less +behaves like an implied # "missingok" now, which isn't correctly parsed in +the current version. Fixing it properly is a very intrusive change. +(LP: #256891) Thanks to Jean-Baptiste Lallement for analyzing the problem +and proposing the patch! + + -- Martin Pitt <[EMAIL PROTECTED]> Wed, 08 Oct 2008 13:59:23 +0200 + logrotate (3.7.1-3ubuntu1) intrepid; urgency=low * Drop mailx to Suggests for Ubuntu; it's only used on request, and we diff -u logrotate-3.7.1/debian/patches/series logrotate-3.7.1/debian/patches/series --- logrotate-3.7.1/debian/patches/series +++ logrotate-3.7.1/debian/patches/series @@ -24,0 +25 @@ +glob-failure.patch only in patch2: unchanged: --- logrotate-3.7.1.orig/debian/patches/glob-failure.patch +++ logrotate-3.7.1/debian/patches/glob-failure.patch @@ -0,0 +1,19 @@ +# Prevent logrotate from failing completely if one glob in any +# included file fails. This more or less behaves like an implied +# "missingok" now, which isn't correctly parsed in the current +# version. +# Ubuntu: https://bugs.launchpad.net/bugs/256891 +# Debian: http://bugs.debian.org/277652 +Index: logrotate-3.7.1/config.c +=== +--- logrotate-3.7.1.orig/config.c 2008-10-08 13:56:36.0 +0200 logrotate-3.7.1/config.c 2008-10-08 13:56:45.0 +0200 +@@ -968,7 +968,7 @@ + + message(MESS_ERROR, "%s:%d glob failed for %s\n", + configFile, lineNum, argv[argNum]); +- return 1; ++ continue; + } + + newlog->files = realloc(newlog->files, sizeof(*newlog->files) * signature.asc Description: Digital signature
