local mailbox %s wrong in mailbox list in cyrus 2.3.15

[Bron Gondwana]

On Sat, Mar 27, 2010 at 05:45:13PM -0500, Dan White wrote:
> On 28/03/10 11:29 +1300, karen turner wrote:
> >Has anyone else had this message in their cyrus logs?
> >
> >Mar 23 14:56:41 zysv01 mupdate[19515]: local mailbox user.zykdt0 wrong in 
> >mailbox list
> >
> >I have identified the section of code that does it.  It's the master mupdate 
> >server synchronization. It is checking for a ! on a local mailbox line of the 
> >murder database.
> >
> >What is the purpose of it checking for a ! on a local mail box line?  
> >A ! is used as a separator, when specifying a remote server, when its a remote 
> >mailbox.
> 
> I have not seen that error, but a ! in the local mailbox list (ctl_mboxlist
> -d) denotes the existence of a virtual domain, e.g.:
> 
> example.org!user.kerri

That's what I call a dumb design decision :(  Using the same separator for
two different purposes in two different contexts.  There's too much of this
in all sorts of places in Cyrus - from using the same variable name as both
a struct and a pointer to said struct through to "clever" stuff like this:

unsigned newseen = 0, newnext = 0;
...
    newseen = (*new == ':');
...
    newnext += newseen;

When what they mean is:

    if (newseen) newnext++;

It's just ugly type coercion.  Yes a boolean result is "1" if true, but ouch.

There's also lots of using the same temporary variable or even the same array
in two different context without a lot of clarity about when/where it has each
meaning.

Like this gem in index_search:

    for (i = 0; i < n; i++) {
        msgno_list[i] = UID(msgno_list[i]);
    }

I can see why, it save a little bit of re-allocating memory for two different
meanings of lists.  But icky variable meaning reuse.

Bron ( rewriting that section of code anyway, but just understanding why was
       not a pleasant experience! )