Re: [PATCH] Remove various dead assignments and dead increments found by the clang static analyzer

[Reece Dunn <msclrhd@googlemail.com>]

2009/9/26 Jeff King <peff@peff.net>:
> 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