On Mon, May 09, 2016 at 06:42:33PM -0500, Derek Martin wrote: > On Sat, May 07, 2016 at 04:14:36AM -0400, Damien Riegel wrote: > > > * The "probe" design: > > > In theory this sounds good, but I don't know that the trade-off is worth > > > it. You still have a single "mx_register_all()" function with #ifdefs > > > in it to put together the linked list. But now, you have to loop > > > through one-by-one, performing multiple stats and such until you find > > > the right one. > > > > Because that means mx.c would have some kind of knowledge of all the > > mailboxes it supports. It is yet another switch case or if statement > > that would need to be modified when adding a new kind of mailbox. I > > think multiple calls to stat have a very minimal perfomance impact, so > > it doesn't really matter for now. At some later point, mx_get_magic > > should be split, and its content dispatched in the different probe > > functions. But I didn't want to make the patchset larger with such > > cleanup commits. > > OK, I get it now. I think this could work OK if the parent did a > single stat, and perhaps did an opendir if the object was a directory, > and then passed copies/pointers/whatever of those objects into the > registered probe functions (pointers for sure). There's no need to do > multiple stats. Then at least the only efficiency loss is the extra > function calls. If dir, the probe function should assume it needs to > do rewinddir(). > > This of course only makes sense if the mail store is a file on a > local file system (or a networked file system that looks local to the > user). I still didn't get through all the patches, not sure how > this is meant to work with non-file mail stores. But that's my $.02!
Thanks for your input Derek. My code was based on the assumption that mx_get_magic is idempotent, but maybe it is not true. I think I will drop changes related to "probe" for now, and simply rely on a switch case to return the correct mx_ops. The probing logic can still be reintroduced later, when the rest of the mailbox operations will be factorized. Thanks, -- Damien
