index.c API (was: Re: Looking for testing volunteers)

[Bron Gondwana]

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?

Bron.