Re: [PATCH] Close config file handle if the entry to unset is not found

From: Eric Sunshine <sunshine@sunshineco.com>
To: Sven Strickroth <sven@cs-ware.de>
Cc: Git List <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] Close config file handle if the entry to unset is not found
Date: Fri, 14 Aug 2015 15:35:39 -0400	[thread overview]
Message-ID: <CAPig+cQMng-bPjO3WLYb1z7aghYhhHqewSr+EGFgV6Gs6_JZRQ@mail.gmail.com> (raw)
In-Reply-To: <55CE3F23.7040702@cs-ware.de>

On Fri, Aug 14, 2015 at 3:18 PM, Sven Strickroth <sven@cs-ware.de> wrote:
> Without this patch there might be open handle leaks.

Thanks for the patch. A question below...

> Signed-off-by: Sup Yut Sum <ch3cooli@gmail.com>
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---
> diff --git a/config.c b/config.c
> index 9fd275f..89b49e3 100644
> --- a/config.c
> +++ b/config.c
> @@ -2048,6 +2048,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
>                 if ((store.seen == 0 && value == NULL) ||
>                                 (store.seen > 1 && multi_replace == 0)) {
>                         ret = CONFIG_NOTHING_SET;
> +                       close(in_fd);
>                         goto out_free;

>From a cursory read of the code, it appears that there are several
other places where the open 'in_fd' gets leaked which would deserve
the same treatment. So, it's not clear why this patch handles only
this one case. Am I missing something?

>                 }
>
> --