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). A better solution would be
const char *newdest = NULL;
...
newdest = buf_cstring(&buf);
...
buf_free(&buf);
}
I thought the same thing when I looked as what was already in master
when I began my forward port:
replyto = buf_release(&buf);
if (body) {
/* replace the existing header */
spool_replace_header(xstrdup("Reply-To"), replyto,
m->hdrcache);
} else {
/* add the new header to the cache */
spool_cache_header(xstrdup("Reply-To"), replyto,
m->hdrcache);
}
} else {
/* no newspostuser, use original replyto */
replyto = (char *) body[0];
}
Was this also incorrect?
Actually, looking at your changes to spool.c (using strarray_t) and
nntpd.c (using struct buf), I think that we DO want to use buf_release()
because spool_cache_header() and spool_replace_header() both use
strarray_appendm() to assign the existing pointer into the strarray
(rather than making a copy) which is later free'd with strrarray_free()
via spool_free_hdrcache().
Am I missing something?
--
Kenneth Murchison
Principal Systems Software Engineer
Carnegie Mellon University
