threads / patch / 21371

patchFix resource leaks in wrapper.c

Subject: [PATCH] Fix resource leaks in wrapper.c

## tl;dr

4 messages between Oct 27, 2009 and Oct 27, 2009. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Laszlo Papp· Oct 27, 2009, 03:53 UTC · lore

Fix the following issues with the desired close tags:

[wrapper.c:276]: (error) Resource leak: fd [wrapper.c:291]: (error) Resource leak: fd

Signed-off-by: Laszlo Papp <djszapi@archlinux.us>
---
 wrapper.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Show changes to wrapper.c +2 −2

diff --git a/wrapper.c b/wrapper.c
index c9be140..76ecf0a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -266,7 +266,7 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern)
 	fd = mkstemp(template);
 	if (0 <= fd)
 		return fd;
-
+	close(fd);
 	/* slow path */
 	/* some mkstemp implementations erase template on failure */
 	snprintf(template, limit, "%s/%s",
@@ -284,7 +284,7 @@ int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)
 	fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
 	if (0 <= fd)
 		return fd;
-
+	close(fd);
 	/* slow path */
 	safe_create_leading_directories(name);
 	return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);

-- 
1.6.5

Johannes Sixt· Oct 27, 2009, 07:13 UTC · re: Laszlo Papp · lore

Re: [PATCH] Fix resource leaks in wrapper.c

Laszlo Papp schrieb:

Show 6 quoted lines

> @@ -266,7 +266,7 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern)
>  	fd = mkstemp(template);
>  	if (0 <= fd)
>  		return fd;
> -
> +	close(fd);

Sorry, where is here a resource leak? You are "closing" something that was never opened because fd is less than zero.

Ditto for the other case.

-- Hannes

Michael J Gruber· Oct 27, 2009, 08:26 UTC · re: Johannes Sixt · lore

Re: [PATCH] Fix resource leaks in wrapper.c

Johannes Sixt venit, vidit, dixit 27.10.2009 08:13:

Show 12 quoted lines

> Laszlo Papp schrieb:
>> @@ -266,7 +266,7 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern)
>>  	fd = mkstemp(template);
>>  	if (0 <= fd)
>>  		return fd;
>> -
>> +	close(fd);
> 
> Sorry, where is here a resource leak? You are "closing" something that was
> never opened because fd is less than zero.
> 
> Ditto for the other case.

I guess it's about silencing some challenged code analysis tool. I recall that last time we had something like this we decided that coders are smarter than tools... and also that clean up like this (for real leaks) would be something for libgit.

Michael

Michael J Gruber· Oct 27, 2009, 11:44 UTC · lore

Re: [PATCH] Fix resource leaks in wrapper.c

Laszlo Papp venit, vidit, dixit 27.10.2009 11:35:

Show 31 quoted lines

> 
> 
> On Tue, Oct 27, 2009 at 9:26 AM, Michael J Gruber
> <git@drmicha.warpmail.net <mailto:git@drmicha.warpmail.net>> wrote:
> 
>     Johannes Sixt venit, vidit, dixit 27.10.2009 08:13:
>     > Laszlo Papp schrieb:
>     >> @@ -266,7 +266,7 @@ int odb_mkstemp(char *template, size_t limit,
>     const char *pattern)
>     >>      fd = mkstemp(template);
>     >>      if (0 <= fd)
>     >>              return fd;
>     >> -
>     >> +    close(fd);
>     >
>     > Sorry, where is here a resource leak? You are "closing" something
>     that was
>     > never opened because fd is less than zero.
>     >
>     > Ditto for the other case.
> 
>     I guess it's about silencing some challenged code analysis tool. I
>     recall that last time we had something like this we decided that coders
>     are smarter than tools... and also that clean up like this (for real
>     leaks) would be something for libgit.
> 
>     Michael
> 
> 
> Yeah you're rights guys, sorry for my fault, this cppcheck program is
> not the best at this momment, really sorry.

No need to feel overly sorry, but in general it helps if, in a commit message or thereabout, you say something like "cppcheck found the following (potential) errors".

Cheers, Michael