Another day, another cyrus bug :(

Wesley Craig wes at umich.edu
Tue Jan 6 17:12:13 EST 2009


On 05 Jan 2009, at 23:38, Bron Gondwana wrote:
> mboxlist_lookup returns a live pointer to a malloc'ed copy of the
> acl.  So far so good.
>
> Except (I presume to reduce memory management effort for callers of
> the function) this value is overwritten next time you call
> mboxlist_lookup again.
>
> So - user_renameacl was clever.  It got the acl and proceeded to
> replace the \t values with \0, and pass through the "rights" string
> to mboxlist_setacl.
>
> Which promptly called mboxlist_lookup AGAIN.

This fix looks fine to me.  Will you commit it?

> The better fix would be a less insanely dangerous API with
> action-at-a-distance on existing copies of the string.  C gives you
> enough rope to shoot yourself in the foot (excuse my metaphores), we
> don't need to help it out!

I agree.  Kind of looks like a case of premature optimization: the  
author was worried about findall cases where mylookup was going to be  
called 1M times or so.  In the absence of a note about how this kind  
of memory optimization is necessary for some case, on some platform,  
or something, I'd be OK seeing the API simplified.

:wes


More information about the Cyrus-devel mailing list