# [PATCH] Fix config_file file leak.

4 messages from 2011-07-14 to 2011-07-20. Participants: Chris Wilson, Ramsay Jones.
Thread: https://gitlist.dev/t/27817

## Chris Wilson, 2011-07-14 18:19

Subject: [PATCH] Fix config_file file leak.
Message-ID: <20110714181948.GA23288@localhost>
URL: https://gitlist.dev/e/20110714181948.GA23288%40localhost

```
Hi,

We are using Sentry (a C/C++ static analysis tool) to analyze
git on a nightly basis. Sentry found that a file leak
was recently introduced in the commit 924aaf3.

I'm hoping the attached patch correctly fixes up this leak.

Thanks,
Chris

-- 
Chris Wilson
http://vigilantsw.com/
Vigilant Software, A C/C++ Static Analysis Company

---
 config.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 1fc063b..bf61f09 100644
--- a/config.c
+++ b/config.c
@@ -1434,7 +1434,7 @@ int git_config_rename_section(const char *old_name, const char *new
        struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
        int out_fd;
        char buf[1024];
-       FILE *config_file;
+       FILE *config_file = 0;

        if (config_exclusive_filename)
                config_filename = xstrdup(config_exclusive_filename);
@@ -1498,12 +1498,13 @@ int git_config_rename_section(const char *old_name, const char *n
                        goto out;
                }
        }
-       fclose(config_file);
  unlock_and_out:
        if (commit_lock_file(lock) < 0)
                ret = error("could not commit config file %s", config_filename);
  out: 
        free(config_filename);
+        if (config_file)
+               fclose(config_file);
        return ret;
 }

-- 
1.7.0.4

```

## Chris Wilson, 2011-07-14 18:29

Subject: Re: [PATCH] Fix config_file file leak.
Message-ID: <20110714182919.GB23288@localhost>
URL: https://gitlist.dev/e/20110714182919.GB23288%40localhost
In-Reply-To: <20110714181948.GA23288@localhost>

```
On Thu, Jul 14, 2011 at 02:19:48PM -0400, Chris Wilson wrote:
> Hi,
> 
> We are using Sentry (a C/C++ static analysis tool) to analyze
> git on a nightly basis. Sentry found that a file leak
> was recently introduced in the commit 924aaf3.
> 
> I'm hoping the attached patch correctly fixes up this leak.

Oops, sorry if I was unclear. This happens in config.c here,
http://git.kernel.org/?p=git/git.git;a=blob;f=config.c;h=1fc063b2562101687b9215e5b697a91fcffdd5bb;hb=924aaf3ef764a5e8e976f68e024ecacf54ff6306
  
This happens once the file is opened successfully here,
  1449         if (!(config_file = fopen(config_filename, "rb"))) {
and then you enter the while loop here,
  1454         while (fgets(buf, sizeof(buf), config_file)) {
and then you take any 'goto out;' in the while loop, which doesn't
close the file handle.

1505  out:
1506         free(config_filename);
1507         return ret;
1508 }


Chris

```

## Ramsay Jones, 2011-07-19 17:28

Subject: Re: [PATCH] Fix config_file file leak.
Message-ID: <4E25BEDB.6040002@ramsay1.demon.co.uk>
URL: https://gitlist.dev/e/4E25BEDB.6040002%40ramsay1.demon.co.uk
In-Reply-To: <20110714181948.GA23288@localhost>

```
Chris Wilson wrote:
> Hi,
> 
> We are using Sentry (a C/C++ static analysis tool) to analyze
> git on a nightly basis. Sentry found that a file leak
> was recently introduced in the commit 924aaf3.

Hmmm ..., commit 924aaf3 did *not* introduce a file handle leak.
It would seem that the change in scope of the file handle made it
easier for Sentry to see the *existing* (potential) file handle leak.

no?

Other than that, ...

ATB,
Ramsay Jones

```

## Chris Wilson, 2011-07-20 13:12

Subject: Re: [PATCH] Fix config_file file leak.
Message-ID: <20110720131214.GC25822@localhost>
URL: https://gitlist.dev/e/20110720131214.GC25822%40localhost
In-Reply-To: <4E25BEDB.6040002@ramsay1.demon.co.uk>

```
On Tue, Jul 19, 2011 at 06:28:59PM +0100, Ramsay Jones wrote:
> Chris Wilson wrote:
> > Hi,
> > 
> > We are using Sentry (a C/C++ static analysis tool) to analyze
> > git on a nightly basis. Sentry found that a file leak
> > was recently introduced in the commit 924aaf3.
> 
> Hmmm ..., commit 924aaf3 did *not* introduce a file handle leak.
> It would seem that the change in scope of the file handle made it
> easier for Sentry to see the *existing* (potential) file handle leak.
> 
> no?

Ah yes, that does seem like that case. Thanks for pointing that out.

> Other than that, ...

Looks like this file handle will leak whenever a write fails.

Chris

```