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
