On 19/08/11 05:54, Florian Pflug wrote:
On Aug17, 2011, at 20:25 , Greg Banks wrote:
On 18/08/11 07:26, Greg Banks wrote:
Incidentally, there is a bugzilla for this
https://bugzilla.cyrusimap.org/show_bug.cgi?id=2924
with a very old patch from Florian Pflug.
Dmitry, if you're able to contribute perhaps you could update that patch for
the 2.4 and master branches, and also remove some of the duplicated
syslog+stderr logging calls? I'm sure it would be much appreciated.
Here's an updated version of that patch rebased onto 2.4. I'm using this on a
production machine and haven't had problems so far, so I'm reasonably confident
that it works as designed.
I've also attach this to the bugzilla entry.
Thanks Florian, the patch looks good except for three problems.
Firstly, the use of LOG_PERROR is unconditional. This is great for
commandline utilities but means that any Cyrus daemon will be emitting
lots of messages to stderr which it did not before. Some init scripts
setups may have difficulty with this. I just checked with someone who
knows the Fastmail code better than I, and Fastmail's scripts are such.
So I'd be a whole bunch more comfortable if the use of LOG_PERROR were
optional, default off, in daemons.
Secondly, I note you're using LOG_PERROR unconditionally in the master
process. However, unless master is started with -D then fd 2 is going
to point at /dev/null and writing syslog messages there will be just a
waste of time. I'm guessing your patch is designed to work with -D, but
the !-D case also needs to be sensible.
Thirdly, can you please also fix the cases where duplicate calls to
syslog and fprintf(stderr) are used, which after your patch will emit a
total of three copies of the message. For example
370 if (r2) {
371 syslog(LOG_ERR, "DBERROR: archive %s: %s", dirname,
372 cyrusdb_strerror(r2));
373 fprintf(stderr,
374 "ctl_cyrusdb: unable to archive environment\n");
375 }
--
Greg.
