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

From: René Scharfe <l.s.r@web.de>
Date: Aug 12, 2014, 16:38 UTC
Message-ID: <53EA430E.8050905@web.de>
In-Reply-To: <1407791481-17410-1-git-send-email-stefanbeller@gmail.com>

Am 11.08.2014 um 23:11 schrieb Stefan Beller:

Show 5 quoted lines

> 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.

Show 19 quoted lines

> 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);
>