PATCH: Optimize add_archive_element call

Dave Korn dave.korn.cygwin@gmail.com
Sun Dec 12 08:00:00 GMT 2010


On 12/12/2010 06:56, H.J. Lu wrote:
> On Sat, Dec 11, 2010 at 8:02 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Fri, Dec 10, 2010 at 03:13:40PM -0800, H.J. Lu wrote:
>>> On Fri, Dec 10, 2010 at 2:37 PM, Alan Modra <amodra@gmail.com> wrote:
>>>> On Fri, Dec 10, 2010 at 05:48:44AM -0800, H.J. Lu wrote:
>>>>> Hi,
>>>>>
>>>>> Before we call add_archive_element, we set subsbfd to NULL and
>>>>> check if it is changed after add_archive_element returns.  This
>>>>> patch sets subsbfd to the unchanged value and avoids the check.
>>>>> OK to install?
>>>> Why not get rid of subsbfd entirely?
>>>>
>>>>          if (! (*info->callbacks->add_archive_element)
>>>>                                (info, element, symdef->name, &element))
>>>>
>>> Like this?
>> Yes, but the same in ldlang.c too.  It's true that "member" is used
>> later, but I think that is a bug.  Better to report an error against
>> the actual file that caused the problem.
>>
>> Like the following, which does the same for other backends too.
>> Also fixes freeing of external symbols when !keep_memory, which may
>> lead to a real bug for xcoff.  The formatting changes are so that
>> emacs c-indent-exp or other automatic indenting keeps the code looking
>> reasonable.
>>
> 
> Please note
> 
> member = bfd_openr_next_archived_file (entry->the_bfd, member);
> 
> If member is changed by plugin, how will it find the next member?

  Also, uninitialised use!

       if (entry->whole_archive)
 	{
-	  bfd *member = NULL;
 	  bfd_boolean loaded = TRUE;

 	  for (;;)
 	    {
-	      bfd *subsbfd;
+	      bfd *member;
+
 	      member = bfd_openr_next_archived_file (entry->the_bfd, member);

    cheers,
      DaveK



More information about the Binutils mailing list