2009/9/26 Jeff King <peff@xxxxxxxx>:
> On Sat, Sep 26, 2009 at 09:15:58PM +0200, Giuseppe Scrivano wrote:
>
>> Here is a cleaned patch. I think these assignments can be removed
>> without any problem.
>
>> --- a/builtin-receive-pack.c
>> +++ b/builtin-receive-pack.c
>> @@ -368,7 +368,7 @@ static char update_post_hook[] = "hooks/post-update";
>> static void run_update_post_hook(struct command *cmd)
>> {
>> struct command *cmd_p;
>> - int argc, status;
>> + int argc;
>> const char **argv;
>>
>> for (argc = 0, cmd_p = cmd; cmd_p; cmd_p = cmd_p->next) {
>> @@ -391,7 +391,7 @@ static void run_update_post_hook(struct command *cmd)
>> argc++;
>> }
>> argv[argc] = NULL;
>> - status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
>> + run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
>> | RUN_COMMAND_STDOUT_TO_STDERR);
>> }
>
> Now this is one that I do think is sensible. The variable isn't used, so
> don't even bother declaring it.
The status variable is removed in this patch.
But then shouldn't the status returned be checked and acted on? That
is, are failures from run_command_v_opt being reported to the user, or
otherwise reacted to?
In this case, IIUC, the status should be returned by the
run_update_post_hook function. I.e.:
- static void run_update_post_hook(struct command *cmd)
+ static int run_update_post_hook(struct command *cmd)
{
struct command *cmd_p;
- int argc, status;
+ int argc;
...
- status = run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
+ return run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
| RUN_COMMAND_STDOUT_TO_STDERR);
}
Thus having the same effect (removing the status variable). Callers of
run_update_post_hook should be checked as well, as should other
run_command_* calls.
- Reece
--
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