[PATCH] convert-dtsv0-lexer.l: fix memory leak

[PATCH] convert-dtsv0-lexer.l: fix memory leak

[Jean-Christophe Dubois]

CID 132822 (#1 of 1): Resource leak (RESOURCE_LEAK)
9. leaked_storage: Variable newname going out of scope leaks the storage it points to

Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>
---
 convert-dtsv0-lexer.l | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
index 259e527..8d8289b 100644
--- a/convert-dtsv0-lexer.l
+++ b/convert-dtsv0-lexer.l
@@ -208,6 +208,12 @@ static void convert_file(const char *fname)
 	char *newname;
 
 	newname = xmalloc(len + sizeof(suffix));
+
+	if (!newname) {
+		die("failed to allocate memory for input file %s: %s\n",
+		    fname, strerror(errno));
+	}
+
 	memcpy(newname, fname, len);
 	memcpy(newname + len, suffix, sizeof(suffix));
 
@@ -223,6 +229,8 @@ static void convert_file(const char *fname)
 
 	while(yylex())
 		;
+
+	free(newname);
 }
 
 int main(int argc, char *argv[])
-- 
2.7.4

Re: [PATCH] convert-dtsv0-lexer.l: fix memory leak

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]

On Wed, Jul 13, 2016 at 12:35:50AM +0200, Jean-Christophe Dubois wrote:
> CID 132822 (#1 of 1): Resource leak (RESOURCE_LEAK)
> 9. leaked_storage: Variable newname going out of scope leaks the storage it points to
> 
> Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>
> ---
>  convert-dtsv0-lexer.l | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
> index 259e527..8d8289b 100644
> --- a/convert-dtsv0-lexer.l
> +++ b/convert-dtsv0-lexer.l
> @@ -208,6 +208,12 @@ static void convert_file(const char *fname)
>  	char *newname;
>  
>  	newname = xmalloc(len + sizeof(suffix));
> +
> +	if (!newname) {
> +		die("failed to allocate memory for input file %s: %s\n",
> +		    fname, strerror(errno));
> +	}
> +

This hunk is unnecessary.  xmalloc() already die()s rather than failing.

>  	memcpy(newname, fname, len);
>  	memcpy(newname + len, suffix, sizeof(suffix));
>  
> @@ -223,6 +229,8 @@ static void convert_file(const char *fname)
>  
>  	while(yylex())
>  		;
> +
> +	free(newname);
>  }
>  
>  int main(int argc, char *argv[])

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] convert-dtsv0-lexer.l: fix memory leak

[Jean-Christophe DUBOIS]

Le 13/07/2016 04:04, David Gibson a écrit :
> On Wed, Jul 13, 2016 at 12:35:50AM +0200, Jean-Christophe Dubois wrote:
>> CID 132822 (#1 of 1): Resource leak (RESOURCE_LEAK)
>> 9. leaked_storage: Variable newname going out of scope leaks the storage it points to
>>
>> Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>
>> ---
>>   convert-dtsv0-lexer.l | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
>> index 259e527..8d8289b 100644
>> --- a/convert-dtsv0-lexer.l
>> +++ b/convert-dtsv0-lexer.l
>> @@ -208,6 +208,12 @@ static void convert_file(const char *fname)
>>   	char *newname;
>>   
>>   	newname = xmalloc(len + sizeof(suffix));
>> +
>> +	if (!newname) {
>> +		die("failed to allocate memory for input file %s: %s\n",
>> +		    fname, strerror(errno));
>> +	}
>> +
> This hunk is unnecessary.  xmalloc() already die()s rather than failing.
OK.

On a correctness point of view there is still the "free" to be done (see 
below).
>
>>   	memcpy(newname, fname, len);
>>   	memcpy(newname + len, suffix, sizeof(suffix));
>>   
>> @@ -223,6 +229,8 @@ static void convert_file(const char *fname)
>>   
>>   	while(yylex())
>>   		;
>> +
>> +	free(newname);
>>   }
>>   
>>   int main(int argc, char *argv[])

Re: [PATCH] convert-dtsv0-lexer.l: fix memory leak

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On Wed, Jul 13, 2016 at 08:02:27AM +0200, Jean-Christophe DUBOIS wrote:
> Le 13/07/2016 04:04, David Gibson a écrit :
> > On Wed, Jul 13, 2016 at 12:35:50AM +0200, Jean-Christophe Dubois wrote:
> > > CID 132822 (#1 of 1): Resource leak (RESOURCE_LEAK)
> > > 9. leaked_storage: Variable newname going out of scope leaks the storage it points to
> > > 
> > > Signed-off-by: Jean-Christophe Dubois <jcd-WBS85hRCVJbxB9160cZjhg@public.gmane.org>
> > > ---
> > >   convert-dtsv0-lexer.l | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
> > > index 259e527..8d8289b 100644
> > > --- a/convert-dtsv0-lexer.l
> > > +++ b/convert-dtsv0-lexer.l
> > > @@ -208,6 +208,12 @@ static void convert_file(const char *fname)
> > >   	char *newname;
> > >   	newname = xmalloc(len + sizeof(suffix));
> > > +
> > > +	if (!newname) {
> > > +		die("failed to allocate memory for input file %s: %s\n",
> > > +		    fname, strerror(errno));
> > > +	}
> > > +
> > This hunk is unnecessary.  xmalloc() already die()s rather than failing.
> OK.
> 
> On a correctness point of view there is still the "free" to be done (see
> below).

Yes.  That was a suggestion to respin.  Never mind now though, I've
removed the unnecessary hunk and applied.

> > 
> > >   	memcpy(newname, fname, len);
> > >   	memcpy(newname + len, suffix, sizeof(suffix));
> > > @@ -223,6 +229,8 @@ static void convert_file(const char *fname)
> > >   	while(yylex())
> > >   		;
> > > +
> > > +	free(newname);
> > >   }
> > >   int main(int argc, char *argv[])
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]