git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] receive-pack: simplify run_update_post_hook()

From
René Scharfe <l.s.r@web.de>
Date
Mar 17, 2017, 23:21 UTC
Message-ID
<f61a1089-833b-2883-deab-f97cb77cef96@web.de>
In-Reply-To
<20170317222320.nsg3yk3nopjadr5t@sigill.intra.peff.net>
Am 17.03.2017 um 23:23 schrieb Jeff King:
Show 36 quoted lines
> 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.

Sure, that's even simpler. I don't know how often the no-args branch would be taken and if the extra allocation would even matter -- that's why I tried to avoid it -- but probably the answers are not often and not much. The only test case that hits it is for the deletion of a non-existent ref.

René
Previous: Jeff KingNext: Junio C Hamano
Message 3 of 6 in “receive-pack: simplify run_update_post_hook()”
  1. receive-pack: simplify run_update_post_hook()René Scharfe, Mar 17, 2017
  2. Jeff KingMar 17, 2017
  3. René ScharfeMar 17, 2017
  4. Junio C HamanoMar 18, 2017
  5. Jeff KingMar 20, 2017
  6. Jonathan NiederMar 17, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.