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