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