On Sun, Apr 13, 2014 at 11:55:54PM +0200, Christian Brueffer wrote: > On 4/13/14 11:46 PM, Konstantin Belousov wrote: > > On Sun, Apr 13, 2014 at 09:23:16PM +0000, Christian Brueffer wrote: > >> Author: brueffer > >> Date: Sun Apr 13 21:23:15 2014 > >> New Revision: 264422 > >> URL: http://svnweb.freebsd.org/changeset/base/264422 > >> > >> Log: > >> Free buf after usage. > >> > >> CID: 1199377 > >> Found with: Coverity Prevent(tm) > >> MFC after: 1 week > >> > >> Modified: > >> head/sys/kern/imgact_elf.c > >> > >> Modified: head/sys/kern/imgact_elf.c > >> ============================================================================== > >> --- head/sys/kern/imgact_elf.c Sun Apr 13 21:13:33 2014 > >> (r264421) > >> +++ head/sys/kern/imgact_elf.c Sun Apr 13 21:23:15 2014 > >> (r264422) > >> @@ -1746,8 +1746,10 @@ __elfN(note_threadmd)(void *arg, struct > >> size = 0; > >> __elfN(dump_thread)(td, buf, &size); > >> KASSERT(*sizep == size, ("invalid size")); > >> - if (size != 0 && sb != NULL) > >> + if (size != 0 && sb != NULL) { > >> sbuf_bcat(sb, buf, size); > >> + free(buf, M_TEMP); > >> + } > >> *sizep = size; > >> } > >> > > Why conditioning free() on size != 0 ? > > IMO free(buf) must be done always, since buf is initialized for the case > > when malloc() is not called. > > > > The corresponding malloc() call is behind a similar conditional, so it > seemed good to be symmetrical here (since buf if NULL otherwise anyway). > > I can change it to call free() unconditionally if you prefer, I don't > mind either way.
Yes, I think that unconditional call would be better, due to strange
interface of dump_thread. It could modify the size, but for now this
is effectively disallowed by KASSERT().
Also, while there, I would change the code
buf = NULL;
if (size != 0 && sb != NULL)
buf = malloc(size, M_TEMP, M_ZERO | M_WAITOK);
to avoid useless initialization if buf, like this:
if (size != 0 && sb != NULL)
buf = malloc(size, M_TEMP, M_ZERO | M_WAITOK);
else
buf = NULL;
pgpYKSHBRB40i.pgp
Description: PGP signature
