Cyrus reviews

[Bron Gondwana]

On Wed, Jan 25, 2012 at 11:16:45AM +1100, Greg Banks wrote:
> On Tue, Jan 24, 2012, at 08:16 AM, Ken Murchison wrote:
> > Greg Banks wrote:
> > > This code
> > > 
> > > +	newdest = buf_release(&buf);
> > > 
> > > will leak the string, as newdest is never free()d (and indeed in some other branches
> > > of the logic, cannot be).  
> > I thought the same thing when I looked as what was already in master 
> > when I began my forward port:
> > [...]
> > Was this also incorrect?
> 
> No, it was correct (if confusing) and is still correct.  The memory pointed to
> by 'newdest' is taken over by either spool_cache_header() or spool_replace_header(),
> not copied, which weirdness I had forgotten.  The fact that 'newdest' sometimes
> points to memory already owned by the function and sometimes not is fine as
> all we do with it is fprintf().
> 
> Sorry, false alarm :(

Of course, overly confusing code is also a bug waiting
to happen...

Bron.