Bug 5536 - Multiple meleaks after bfd_arch_list() calls
Summary: Multiple meleaks after bfd_arch_list() calls
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: unspecified
: P2 minor
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-02 21:32 UTC by Jakub Zawadzki
Modified: 2008-01-09 10:44 UTC (History)
1 user (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build: x86_64-pc-linux-gnu
Last reconfirmed:


Attachments
free memory allocated by bfd_arch_list () (806 bytes, patch)
2008-01-08 23:12 UTC, Jakub Zawadzki
Details | Diff
Revised memory leak patch (1.02 KB, patch)
2008-01-09 10:41 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Zawadzki 2008-01-02 21:32:31 UTC
bfd_arch_list() doc

FUNCTION
        bfd_arch_list

SYNOPSIS
        const char **bfd_arch_list (void);

DESCRIPTION
        Return a freshly malloced NULL-terminated vector of the names
        of all the valid BFD architectures.  Do not modify the names.

_Return a freshly malloced_

grep gave me 3 hits of bfd_arch_list() call:

$ grep -Irn 'bfd_arch_list' ./
./bucomm.c:205:  for (arch = bfd_arch_list (); *arch; arch++)
./windmc.c:250:    const char **arch = bfd_arch_list ();
./windres.c:1076:    const char **arch = bfd_arch_list();

Every time we don't save result of bfd_arch_list() and we don't free it.
Comment 1 Andreas Schwab 2008-01-02 23:37:55 UTC
These are all one-time uses, so they don't really leak.
Comment 2 Jakub Zawadzki 2008-01-08 23:08:18 UTC
Yes, sure, it's nothing _very_ important, but I don't know why it shouldn't be
fixed.

In list_supported_targets () [which is also used hardly ever] we free memory
after bfd_target_list () call

I attach patch to fix these memleaks, and change a little list_matching_formats ()
I think it's better.
Comment 3 Jakub Zawadzki 2008-01-08 23:12:46 UTC
Created attachment 2188 [details]
free memory allocated by bfd_arch_list ()
Comment 4 Nick Clifton 2008-01-09 10:41:04 UTC
Created attachment 2190 [details]
Revised memory leak patch
Comment 5 Nick Clifton 2008-01-09 10:44:19 UTC
Hi Jakub, Hi Andreas,

  I agree that there is no reason why we should not fix the leaks, so I have
gone ahead and applied a slight variant of Jakub's patch (uploaded).  The
changes I made were:

  * Omitted the change to list_matching_formats().  There was no need for it
    and any decent compiler will produce the same code either way.

  * Moved the architecture walking variable ('arch') in the set_endianess() 
    functions inside the if-statement.

  * Added a ChangeLog entry.

Cheers
  Nick

binutils/ChangeLog
2008-01-09  Jakub Zawadzki  <darkjames@darkjames.ath.cx>

	PR binutils/55326
	* bucomm.c (list_supported_architectures): Free architecture list
	after use.
	* windres.c (set_endianess): Likewise.
	* windmc.c (set_endianess): Likewise.