cvt_cyrusdb utility does not work if /var/lib/cyrus/db is inconsistent

[Greg Banks]

Sent from my iPhone

On 18/08/2011, at 3:09, Michael Loftis <mloftis at wgops.com> wrote:

> 2011/8/17 Dmitry Katsubo <dma_k at mail.ru>:
>> Dear Cyrus community,
> ......
>> I have digged the code and found the following in
>> lib/cyrusdb_berkeley.c:103:
>>
>> static void db_panic(DB_ENV *dbenv __attribute__((unused)),
>>             int errno __attribute__((unused)))
>> {
>>    syslog(LOG_CRIT, "DBERROR: critical database situation");
>>    /* but don't bounce mail */
>>    exit(EC_TEMPFAIL);
>> }
>>
>> I believe this behaviour is OK for imapd server, but what about  
>> command
>> line utility? In any case having exit() call in such an "unexpected"
>> place (callback!) seems to be not good, as the process terminates
>> suddenly and someone may spend time to find out the actual exit  
>> point. I
>> believe that more correct behaviour would be to raise the error flag,
>> and return the error code from init() function.
>>
>> And what is sad, that the functioning of cvt_cyrusdb depends on many
>> objects, which it does not direclty operate with.
>
>
> Your answer is there, you check the exit code.  If non-zero then there
> was a failure, generic unix scripting rule.

Sure, but Dmitry does have a point that the error message appearing  
only in syslog is an unexpected behaviour for a commandline utility.

This is a widespread issue in Cyrus, where lots of common code was  
clearly designed to live in a server, and it has made adding unit  
tests that little bit more challenging. For example the unit test  
framework now intercepts calls to the exit() libc routine and turns  
that into a longjmp() out to the test failure code. It also intercepts  
syslog() and pattern matches the message so that tests can pass or  
fail depending on the occurrence or not of log messages.

There's also the opposite problem of error handling code which calls  
both syslog and fprintf(stderr), one after the other. That's just  
silly, especially given that many modern syslog implementations have a  
LOG_STDERR flag that will copy all syslog messages to stderr also.



>
> Bonus points for using mktemp, then mv-ing in the sucessful case and
> cleaning up after yourself in the case of failure (hint, using trap
> can save you lots of code here too).

This too is something that it's reasonable to expect a modern  
commandline database utility to do for you.

Greg.
>