Fix config_file file leak.
PATCH 1/1 Fix config_file file leak.
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
Signature · 7 lines
Chris Wilson http://vigilantsw.com/ Vigilant Software, A C/C++ Static Analysis Company --- config.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
config.c +3 -2
@@ -1434,7 +1434,7 @@ int git_config_rename_section(const char *old_name, const char *newstruct 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; }
Signature · 1 line
1.7.0.4
Quoted 7 lines · Chris Wilson
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
Quoted 5 lines · Chris Wilson
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
Quoted 7 lines · Ramsay Jones
Quoted 5 lines · Chris Wilson
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.
Quoted 5 lines
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.
Quoted 1 line
Other than that, ...
Looks like this file handle will leak whenever a write fails.
Chris