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