Handling strings, lists and more complex general datastructures in Cyrus

Bron Gondwana brong at fastmail.fm
Sat Jan 1 08:44:07 EST 2011


I've just been doing some work with the annotations code, which has its
own string list handling libraries.

I've also been working in mupdate, which has its own datastructure to
wrap the mailboxes.db entries.  As did imapd and the annotations code,
until I stripped it out and replaced it with struct mboxlist_entry
everywhere.  I'm planning to do the same to murder as soon as I figure
out how it will interact with mpools.

We have an insane number of separate datastructures and support functions
for handling basic datatypes in Cyrus.  Which is all a hangover from there
not being any one good way to do it!  But they suck variously.

Even more - the ownership of memory is really unclear in places, and there
are piles of small leaks around the place (I've seen one with the partition
name in cmd_create right now for example)

Finally, there are a lot more "complex" datastructures to pass around, for
things like the extended commands from RFC4466, QRESYNC, etc.  We've been
building a pile of custom structs for each of them - but then having to add
allocation and deallocation support functions for them all.

And replication has a pile of datastructure list things too for that matter,
plus the brand new "dlist" thing I added which is kind of like the DOM
interface in javascript/xml in use, but not particularly well defined or
complete yet.

--------------

Basically, it's all a bit messy.  I'd really like to make a more generic
parsing library that can give us an inspectable datastructure that can be
passed around, so that travesties like this can be brought under control:

static int
mboxlist_mycreatemailboxcheck(const char *mboxname,
>------->------->-------      int new_mbtype __attribute__((unused)),
>------->------->-------      const char *partition,
>------->------->-------      int isadmin, const char *userid,
>------->------->-------      struct auth_state *auth_state,
>------->------->-------      char **newacl, char **newpartition,
>------->------->-------      int RMW, int localonly, int force_user_create,
>------->------->-------      struct txn **tid)

int mboxlist_createmailbox(const char *name, int mbtype,
>------->------->-------   const char *partition,
>------->------->-------   int isadmin, const char *userid,
>------->------->-------   struct auth_state *auth_state,
>------->------->-------   int localonly, int forceuser, int dbonly,
>------->------->-------   struct dlist *extargs)


And we can add future extentions without having to reinvent quite
so many wheels.


---------

Greg - I'll try to sit down with you this week and we can identify some
of the datastructures we need to subsume, and make a plan of attack to
remove them.  I'd particularly like to normalise some of the mailbox
create stuff so we can pass down all the facts we know to the create
without the API getting wildly out of control.

Bron ( hacky implementations of METADATA, SPECIAL-USE and
       CREATE-SPECIAL-USE (plus XLIST backwards compatibility) now
       basically done, but I've added enough code stink in the process
       that it's time to sit back a bit and refactor now! )


More information about the Cyrus-devel mailing list