index.c API

Ken Murchison murch at andrew.cmu.edu
Tue Oct 30 23:36:44 EDT 2007


Bron Gondwana wrote:
> On Fri, Oct 26, 2007 at 04:33:16PM -0400, Ken Murchison wrote:
>> Folks,
>>
>> I just make some changes to two vitally important Cyrus source files, that 
>> I'd like to get some independent testing on.  I banged on both sets of 
>> changes myself, but the community can usually find bugs that my testing 
>> didn't reveal.  The changes are mutually exclusive, so they can be tested 
>> independently.  All diffs are against 2.3.10.
>>
>> The first change was to lib/cyrusdb_skiplist.c to squash all of the 
>> signed/unsigned warnings.
>>
>> https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/cyrus/lib/cyrusdb_skiplist.c.diff?r1=1.52;r2=1.53
>>
>>
>> The second change was to imap/index.c.  There are 3 major changes. First, I 
>> removed redundant code in index_parse_sequence().  Second, I refactored 
>> index_forsequence() to use index_parse_sequence().  Third, I squashed all 
>> signed/unsigned warnings.  There are some collateral changes in other 
>> files, but no change in functionality to them.
>>
>> https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/cyrus/imap/index.c.diff?r1=1.232;r2=1.237
>> https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/cyrus/imap/index.h.diff?r1=1.14;r2=1.15
>> https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/cyrus/imap/imapd.c.diff?r1=1.532;r2=1.533
>> https://bugzilla.andrew.cmu.edu/cgi-bin/cvsweb.cgi/src/cyrus/imap/imapd.h.diff?r1=1.66;r2=1.67
> 
> Sorry to take so long to get back on you with these.  I've imported them
> both into my quilt repository and am going to go ahead with testing
> them.
> 
> 
> I also have a bunch of questions for the other stuff I'm doing - first
> is the "index.h/index.c API".  It looks like all the functions in there
> are designed to be used with the following basic pattern:
> 
> index_operatemailbox(&mailbox);
> index_getuid(msgno);
> 
> etc.
> 
> However there are functions like this:
> 
> extern char *index_get_msgid(struct mailbox *mailbox, unsigned msgno);
> 
> Which take a mailbox (unused), or even more funky:
> 
> extern char *index_getheader(struct mailbox *mailbox, unsigned msgno,
>            char *hdr)
> 
> Which takes a mailbox struct and even uses it in some places, yet at
> the same time uses the global cache_base rather than the one in the
> mailbox object, meaning it absolutely requires the same mailbox
> be passed anyway.
> 
> Is there a good reason behind all this?  Can I talk you into making
> these functions "re-entrant" by always requiring a mailbox be passed,
> and hence not storing anything global, or alternatively storing the
> entire mailbox struct globally and referring to that so it never
> needs to be passed?

Without going to the source to look closely, my guess is that those 
functions that don't take a mailbox as an argument were designed to work 
on the currently selected mailbox.  Those that do take a mailbox arg, 
can operate on any mailbox.

It won't take much convincing to straighten this out.  Anything which 
gets us closer to being thread-safe is a good thing IMHO.

-- 
Kenneth Murchison
Systems Programmer
Project Cyrus Developer/Maintainer
Carnegie Mellon University


More information about the Cyrus-devel mailing list