Re: [PATCH 3/5] remove deleted BFDs from the archive cache

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Aug 17, 2012 at 10:11 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "HJ" == H J Lu <hjl.tools@gmail.com> writes:
>
> HJ> +      else
> HJ> +   {
> HJ> +     /* If HTAB is NULL, free ARED allocated with bfd_zmalloc.  */
> HJ> +     free (ared);
> HJ> +   }
>
> HJ> However, it assumes that archive member from filesystem is
> HJ> closed after archive.  It won't be easy to get around it since
> HJ> we can't get from archive member from filesystem to
> HJ> archive.
>
> I prefer my approach, because although my patch is more complicated, the
> resulting code is simpler.  I think this because, after my patch, an
> areltdata is allocated in a single way and also freed in a single way.
> This uniformity makes them easier to reason about.

Given
  archive = bfd_openr (argv[1], NULL);

  for (last = bfd_openr_next_archived_file (archive, NULL);
       last;
       last = next)
    {
      next = bfd_openr_next_archived_file (archive, last);
      bfd_close (last);
    }

will bfd_close (last) call free (abfd->arelt_data)? If it does,
archive bfd will have bad member arelt_data.

> I find the above quite obscure.  It assumes an invariant that is not
> obvious and that is also not documented.
>
> If you go with your patch, I'd recommend you also fix the latent bugs I
> pointed out.  Some of them would require different fixes than what I
> posted.
>

Those latent bugs should be fixed by separate patches.


-- 
H.J.