On Fri, Mar 17, 2017 at 11:02:13PM +0100, René Scharfe wrote:
> Instead of counting the arguments to see if there are any and then
> building the full command use a single loop and add the hook command
> just before the first argument. This reduces duplication and overall
> code size.
Yeah, I agree one loop is nicer.
> - argv_array_push(&proc.args, hook);
> for (cmd = commands; cmd; cmd = cmd->next) {
> if (cmd->error_string || cmd->did_not_exist)
> continue;
> + if (!proc.args.argc)
> + argv_array_push(&proc.args, hook);
> argv_array_push(&proc.args, cmd->ref_name);
> }
> + if (!proc.args.argc)
> + return;
It looks at first like the result leaks, because you have to realize
that the push will modify proc.args.argc. I wonder if:
argv_array_push(&proc.args, hook);
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string && !cmd->did_not_exist)
argv_array_push(&proc.args, cmd->ref_name);
}
if (proc.args.argc == 1) {
argv_array_clear(&proc.args);
return;
}
would be more obvious (at the cost of a pointless malloc in the corner
case. I can live with it either way.
-Peff