* [PATCH] Fix config_file file leak.
@ 2011-07-14 18:19 Chris Wilson
2011-07-14 18:29 ` Chris Wilson
2011-07-19 17:28 ` Ramsay Jones
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2011-07-14 18:19 UTC (permalink / raw
To: git
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix config_file file leak.
2011-07-14 18:19 [PATCH] Fix config_file file leak Chris Wilson
@ 2011-07-14 18:29 ` Chris Wilson
2011-07-19 17:28 ` Ramsay Jones
1 sibling, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2011-07-14 18:29 UTC (permalink / raw
To: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix config_file file leak.
2011-07-14 18:19 [PATCH] Fix config_file file leak Chris Wilson
2011-07-14 18:29 ` Chris Wilson
@ 2011-07-19 17:28 ` Ramsay Jones
2011-07-20 13:12 ` Chris Wilson
1 sibling, 1 reply; 4+ messages in thread
From: Ramsay Jones @ 2011-07-19 17:28 UTC (permalink / raw
To: Chris Wilson; +Cc: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix config_file file leak.
2011-07-19 17:28 ` Ramsay Jones
@ 2011-07-20 13:12 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2011-07-20 13:12 UTC (permalink / raw
To: Ramsay Jones; +Cc: git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-07-20 13:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-14 18:19 [PATCH] Fix config_file file leak Chris Wilson
2011-07-14 18:29 ` Chris Wilson
2011-07-19 17:28 ` Ramsay Jones
2011-07-20 13:12 ` Chris Wilson
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).