[RFA] ar.c (delete_members): Plug memory leak.

[RFA] ar.c (delete_members): Plug memory leak.

[Michael Snyder]

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

OK?


[-- Attachment #2: ar5.txt --]
[-- Type: text/plain, Size: 1153 bytes --]

2011-03-08  Michael Snyder  <msnyder@vmware.com>

	* ar.c (delete_members): Plug memory leak.

Index: ar.c
===================================================================
RCS file: /cvs/src/src/binutils/ar.c,v
retrieving revision 1.72
diff -u -p -r1.72 ar.c
--- ar.c	8 Dec 2010 05:05:30 -0000	1.72
+++ ar.c	8 Mar 2011 20:35:36 -0000
@@ -1117,6 +1121,7 @@ delete_members (bfd *arch, char **files_
   bfd_boolean found;
   bfd_boolean something_changed = FALSE;
   int match_count;
+  char *tmp = NULL;
 
   for (; *files_to_delete != NULL; ++files_to_delete)
     {
@@ -1138,8 +1143,9 @@ delete_members (bfd *arch, char **files_
       current_ptr_ptr = &(arch->archive_next);
       while (*current_ptr_ptr)
 	{
-	  if (FILENAME_CMP (normalize (*files_to_delete, arch),
-			    (*current_ptr_ptr)->filename) == 0)
+	  free (tmp);
+	  tmp = normalize (*files_to_delete, arch);
+	  if (FILENAME_CMP (tmp, (*current_ptr_ptr)->filename) == 0)
 	    {
 	      ++match_count;
 	      if (counted_name_mode
@@ -1176,6 +1182,7 @@ delete_members (bfd *arch, char **files_
     write_archive (arch);
   else
     output_filename = NULL;
+  free (tmp);
 }
 
 

Re: [RFA] ar.c (delete_members): Plug memory leak.

[Nick Clifton]

Hi Michael,

> 2011-03-08  Michael Snyder<msnyder@vmware.com>
>
> 	* ar.c (delete_members): Plug memory leak.

Approved and applied.

Cheers
   Nick

Re: [RFA] ar.c (delete_members): Plug memory leak.

[Matthias Klose]

On 03/25/2011 06:18 PM, Nick Clifton wrote:
> Hi Michael,
>
>> 2011-03-08 Michael Snyder<msnyder@vmware.com>
>>
>> * ar.c (delete_members): Plug memory leak.
>
> Approved and applied.

this causes `ar d' to segfault, reverting this change lets ar succeed.
filed PR binutils/12720

   Matthias

Re: [RFA] ar.c (delete_members): Plug memory leak.

[H.J. Lu]

On Tue, Mar 8, 2011 at 12:37 PM, Michael Snyder <msnyder@vmware.com> wrote:
> OK?
>
>
> 2011-03-08  Michael Snyder  <msnyder@vmware.com>
>
>        * ar.c (delete_members): Plug memory leak.
>
> Index: ar.c
> ===================================================================
> RCS file: /cvs/src/src/binutils/ar.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 ar.c
> --- ar.c        8 Dec 2010 05:05:30 -0000       1.72
> +++ ar.c        8 Mar 2011 20:35:36 -0000
> @@ -1117,6 +1121,7 @@ delete_members (bfd *arch, char **files_
>   bfd_boolean found;
>   bfd_boolean something_changed = FALSE;
>   int match_count;
> +  char *tmp = NULL;
>
>   for (; *files_to_delete != NULL; ++files_to_delete)
>     {
> @@ -1138,8 +1143,9 @@ delete_members (bfd *arch, char **files_
>       current_ptr_ptr = &(arch->archive_next);
>       while (*current_ptr_ptr)
>        {
> -         if (FILENAME_CMP (normalize (*files_to_delete, arch),
> -                           (*current_ptr_ptr)->filename) == 0)
> +         free (tmp);
> +         tmp = normalize (*files_to_delete, arch);
> +         if (FILENAME_CMP (tmp, (*current_ptr_ptr)->filename) == 0)
>            {
>              ++match_count;
>              if (counted_name_mode
> @@ -1176,6 +1182,7 @@ delete_members (bfd *arch, char **files_
>     write_archive (arch);
>   else
>     output_filename = NULL;
> +  free (tmp);
>  }
>
>

The patch is wrong since normalize may not always malloc and
return a memory.  It should be reverted.


-- 
H.J.

Re: [RFA] ar.c (delete_members): Plug memory leak.

[H.J. Lu]

On Sun, May 1, 2011 at 9:04 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Mar 8, 2011 at 12:37 PM, Michael Snyder <msnyder@vmware.com> wrote:
>> OK?
>>
>>
>> 2011-03-08  Michael Snyder  <msnyder@vmware.com>
>>
>>        * ar.c (delete_members): Plug memory leak.
>>
>> Index: ar.c
>> ===================================================================
>> RCS file: /cvs/src/src/binutils/ar.c,v
>> retrieving revision 1.72
>> diff -u -p -r1.72 ar.c
>> --- ar.c        8 Dec 2010 05:05:30 -0000       1.72
>> +++ ar.c        8 Mar 2011 20:35:36 -0000
>> @@ -1117,6 +1121,7 @@ delete_members (bfd *arch, char **files_
>>   bfd_boolean found;
>>   bfd_boolean something_changed = FALSE;
>>   int match_count;
>> +  char *tmp = NULL;
>>
>>   for (; *files_to_delete != NULL; ++files_to_delete)
>>     {
>> @@ -1138,8 +1143,9 @@ delete_members (bfd *arch, char **files_
>>       current_ptr_ptr = &(arch->archive_next);
>>       while (*current_ptr_ptr)
>>        {
>> -         if (FILENAME_CMP (normalize (*files_to_delete, arch),
>> -                           (*current_ptr_ptr)->filename) == 0)
>> +         free (tmp);
>> +         tmp = normalize (*files_to_delete, arch);
>> +         if (FILENAME_CMP (tmp, (*current_ptr_ptr)->filename) == 0)
>>            {
>>              ++match_count;
>>              if (counted_name_mode
>> @@ -1176,6 +1182,7 @@ delete_members (bfd *arch, char **files_
>>     write_archive (arch);
>>   else
>>     output_filename = NULL;
>> +  free (tmp);
>>  }
>>
>>
>
> The patch is wrong since normalize may not always malloc and
> return a memory.  It should be reverted.
>

http://sourceware.org/ml/binutils/2011-03/msg00151.html
http://sourceware.org/ml/binutils/2011-03/msg00154.html
http://sourceware.org/ml/binutils/2011-03/msg00157.html

may have the same issue.


-- 
H.J.

Re: [RFA] ar.c (delete_members): Plug memory leak.

[Alan Modra]

I have reverted the problem patches.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [RFA] ar.c (delete_members): Plug memory leak.

[H.J. Lu]

On Sun, May 1, 2011 at 11:17 PM, Alan Modra <amodra@gmail.com> wrote:
> I have reverted the problem patches.
>

I checked in this patch to add testcases for "ar -r" and "ar -m".


-- 
H.J.
---
diff --git a/binutils/testsuite/ChangeLog b/binutils/testsuite/ChangeLog
index 178b3eb..773070b 100644
--- a/binutils/testsuite/ChangeLog
+++ b/binutils/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2011-05-02  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR binutils/12720
+	* binutils-all/ar.exp (delete_an_element): New.
+	(move_an_element): Likewise.
+	Run delete_an_element and move_an_element.
+
 2011-04-30  H.J. Lu  <hongjiu.lu@intel.com>

 	* binutils-all/x86-64/compressed-1a.d: Adjust for change in output
diff --git a/binutils/testsuite/binutils-all/ar.exp
b/binutils/testsuite/binutils-all/ar.exp
index 4b8a2da..0caa847 100644
--- a/binutils/testsuite/binutils-all/ar.exp
+++ b/binutils/testsuite/binutils-all/ar.exp
@@ -438,6 +438,88 @@ proc unique_symbol { } {
     pass $testname
 }

+# Test deleting an element.
+
+proc delete_an_element { } {
+    global AR
+    global AS
+    global srcdir
+    global subdir
+
+    set testname "ar deleting an element"
+
+    if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.o] {
+	unresolved $testname
+	return
+    }
+
+    if [is_remote host] {
+	set archive artest.a
+	set objfile [remote_download host tmpdir/bintest.o]
+	remote_file host delete $archive
+    } else {
+	set archive tmpdir/artest.a
+	set objfile tmpdir/bintest.o
+    }
+
+    remote_file build delete tmpdir/artest.a
+
+    set got [binutils_run $AR "-r -c $archive ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    set got [binutils_run $AR "-d $archive ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
+# Test moving an element.
+
+proc move_an_element { } {
+    global AR
+    global AS
+    global srcdir
+    global subdir
+
+    set testname "ar moving an element"
+
+    if ![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.o] {
+	unresolved $testname
+	return
+    }
+
+    if [is_remote host] {
+	set archive artest.a
+	set objfile [remote_download host tmpdir/bintest.o]
+	remote_file host delete $archive
+    } else {
+	set archive tmpdir/artest.a
+	set objfile tmpdir/bintest.o
+    }
+
+    remote_file build delete tmpdir/artest.a
+
+    set got [binutils_run $AR "-r -c $archive ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    set got [binutils_run $AR "-m $archive ${objfile}"]
+    if ![string match "" $got] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
 # Run the tests.

 long_filenames
@@ -446,6 +528,8 @@ thin_archive
 thin_archive_with_nested
 argument_parsing
 deterministic_archive
+delete_an_element
+move_an_element
 if { [is_elf_format]
      && ![istarget "*-*-hpux*"]
      && ![istarget "msp*-*-*"] } {