Re: [PATCH 2/3] new: Add all initial tags at once

[Carl Worth <cworth@cworth.org>]

[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]

On Wed, 26 Jan 2011 17:52:53 +0100, Thomas Schwinge <thomas@schwinge.name> wrote:
> I do support the patch's idea (which was recently committed; and what
> follows in this message is not at all directed towards Michal, who wrote
> this patch) -- but what about return values checking?  This is one aspect
> of the notmuch C code (which I generally consider to be nice to read and
> of high quality, as I said before already), that I consider totally
> lacking -- there are literally hundreds of C functions calls where the
> return values are just discarded.  This is bad.  For example (simulating
> a full disk):
> 
>     $ notmuch dump > /dev/full
>     $ echo $?
>     0

All very well pointed out. This is clearly something we need to fix.

> Other languages have the concept of exceptions; C doesn't, so we're
> supposed to put some ``ABORT_IF_NOT_NOTMUCH_STATUS_SUCCESS(ret)''
> statements after each and every non-void (etc.) C function call.  Or make
> the functions abort themselves (which is not a too good idea, as we
> surely agree).  Or use a different programming language -- now, at the
> present state, it wouldn't be too painful to switch, in my opinion.  (I
> won't suggest any specific language, though.)

I wouldn't have any problem with anyone re-implementing notmuch in some
other language than C. But that's not something I would be likely to
work on myself, I don't think. 

>                                                If staying with C (which I
> don't object, either), then this needs a whole code audit, and a lot of
> discipline in the future.

Even a code audit and developer discipline won't be enough here. We'll
still miss things.

What we need is exhaustive testing. A great approach is to take calls
like malloc, open, read, write, etc. and at each site, fork() and fail
the call along one path, (which should then exit with a failure), and
then let the other path continue.

Just a few hours ago I attended an interesting talk by Rusty Russell in
which he talks about a CCAN module he has written called failtest which
provides an implementation of this kind of testing.

I'd love to see something like that integrated with notmuch.

> Comments?  (And I hope this doesn't sound too harsh :-) -- but it is a
> serious programming issue.)

Please don't apologize! It would be a shame if people didn't share
problems they notice in the code. Being able to hear those kinds of
things is one of the great benefits I get from publishing this code as
free software.

So, please, keep the suggestions coming!

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]