Re: [PATCH] receive-pack: plug minor memory leak in unpack()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Sat, Oct 11, 2014 at 01:00:16PM +0200, René Scharfe wrote:
>
>> The argv_array used in unpack() is never freed.  Instead of adding
>> explicit calls to argv_array_clear() use the args member of struct
>> child_process and let run_command() and friends clean up for us.
>
> Looks good. I notice that the recently added prepare_push_cert_sha1 uses
> an argv_array to create the child_process.env, and we leak the result.

You're right.  finish_command() runs argv_array_clear() on
cmd->args, but does not care about cmd->env so anybody that use the
(optional) "use these environment variable settings while running
the command" would leak unless the caller takes care of it.

> I wonder if run-command should provide a managed env array similar
> to the "args" array.

I took a look at a few of them:

 - setup_pager() uses a static set of environment variables that are
   not built with argv_array(); should be easy to convert, though.

 - wt_status_print_submodule_summary() does use two argv_arrays, for
   env and argv.  It can use the managed "args" today, and with such
   a change it can also use the managed "envs".

 - receive-pack.c::prepare_push_cert_sha1() would benefit from
   managed "envs".

 - http-backend.c::run_service() would benefit from managed "envs".

 - daemon.c::handle() uses a static set of environment variables
   that are not built with argv_array().  Same for argv.

It shouldn't be too hard to catch and convert all of them.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]