Subject: Re: [PATCH 08/26] difftool: close file descriptors after reading

From: Junio C Hamano <[email-masked]>
Date: Wed, 26 Apr 2017 23:05:10 -0700

Johannes Schindelin <[email-masked]> writes:

> Spotted by Coverity.
>
> Signed-off-by: Johannes Schindelin <[email-masked]>
> ---
>  builtin/difftool.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/builtin/difftool.c b/builtin/difftool.c
> index 1354d0e4625..a4f1d117ef6 100644
> --- a/builtin/difftool.c
> +++ b/builtin/difftool.c
> @@ -226,6 +226,7 @@ static void changed_files(struct hashmap *result, const char *index_path,
>  		hashmap_entry_init(entry, strhash(buf.buf));
>  		hashmap_add(result, entry);
>  	}
> +	fclose(fp);
>  	if (finish_command(&diff_files))
>  		die("diff-files did not exit properly");
>  	strbuf_release(&index_env);

This one looks sensible.

> @@ -497,6 +498,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
>  		}
>  	}
>  
> +	fclose(fp);

The huge loop we see in the pre-context of this hunk has many
"return"s and "goto finish"es that can leave fp still open; while
this patch does not hurt, it is probably somewhat insufficient.

>  	if (finish_command(&child)) {
>  		ret = error("error occurred running diff --raw");
>  		goto finish;