Re: [PATCH] mailsplit.c: remove dead code

["René Scharfe" <l.s.r@web.de>]

Am 11.08.2014 um 23:11 schrieb Stefan Beller:
> This was found by coverity. (Id: 290001)
>
> the variable 'output' is only assigned to a value inequal to NUL,
> after all gotos to the corrupt label.
> Therefore we can conclude the two removed lines are actually dead code.

After reading the above for the first time I thought it meant the 
opposite of what's actually going on.  Perhaps it's the placement of 
"only", the comma or a flawed understanding of grammar on my part?

In any case, there is only one way to reach the label named corrupt, and 
the variable named output is always NULL if that branch is taken.  That 
means the removed code was a no-op.  With those two lines gone you also 
don't need to initialize output anymore, by the way.

And since there is only a single goto, you could move the three 
remaining error handling lines up to the if statement.  Keeping 
condition and dependent code together would be an improvement, I think.

> Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
> ---
>   builtin/mailsplit.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
> index 06296d4..b499014 100644
> --- a/builtin/mailsplit.c
> +++ b/builtin/mailsplit.c
> @@ -93,8 +93,6 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
>   	return status;
>
>    corrupt:
> -	if (output)
> -		fclose(output);
>   	unlink(name);
>   	fprintf(stderr, "corrupt mailbox\n");
>   	exit(1);
>