Re: [PATCH] Correctly close config file handle in case of error

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 14, 2015, 20:14 UTC
Message-ID: <CAPig+cTo=iiMvvcfRu68feofoBVSFjrd4zG8GYwXN5L6THr=8Q@mail.gmail.com>
In-Reply-To: <55CE49AD.4010605@cs-ware.de>

On Fri, Aug 14, 2015 at 4:03 PM, Sven Strickroth <sven@cs-ware.de> wrote:

Show 15 quoted lines

> Without this patch there might be open file handle leaks.
>
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> Signed-off-by: Sup Yut Sum <ch3cooli@gmail.com>
> ---
> diff --git a/config.c b/config.c
> index 9fd275f..8138d5d 100644
> --- a/config.c
> +++ b/config.c
> @@ -2065,6 +2065,7 @@ int git_config_set_multivar_in_file(const char
> *config_filename,
>                         goto out_free;
>                 }
>                 close(in_fd);
> +               in_fd = -1;

You also need to initialize 'in_fd' to -1 at its point of declaration since there are goto's to 'out_free' which occur before the `in_fd = open(...)'.

Show 12 quoted lines

>                 if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {
>                         error("chmod on %s failed: %s",
> @@ -2148,6 +2149,8 @@ out_free:
>         free(filename_buf);
>         if (contents)
>                 munmap(contents, contents_sz);
> +       if (in_fd >= 0)
> +               close(in_fd);
>         return ret;
>
>  write_err_out:
> --