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.