This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] bfd: Add support for more than one plugin in lib/bfd-plugins


On 2014.09.23 at 21:38 +0930, Alan Modra wrote:
> On Tue, Sep 23, 2014 at 08:25:36AM +0200, Markus Trippelsdorf wrote:
> 
> I like what you're doing, but there are a few problems.
> 
> > +try_load_plugin (const char *pname, bfd *abfd)
> >  {
> >    static void *plugin_handle;
> 
> Nit, preexisting.  Seems to me that this should no longer be a static
> var.

Fixed.

> >    int tv_size = 4;
> > @@ -164,7 +209,6 @@ try_load_plugin (const char *pname)
> >    int i;
> >    ld_plugin_onload onload;
> >    enum ld_plugin_status status;
> > -
> >    plugin_handle = dlopen (pname, RTLD_NOW);
> >    if (!plugin_handle)
> >      {
> > @@ -200,6 +244,9 @@ try_load_plugin (const char *pname)
> >    if (!claim_file)
> >      goto err;
> >  
> > +  if (!try_claim (abfd))
> > +    goto err;
> 
> Since you're calling try_claim here..
> 
> > -  lseek(file.fd, cur_offset, SEEK_SET);
> > -  if (!claimed)
> > +  if (!try_claim (abfd))
> >      return NULL;
> 
> ..you shouldn't do so here in bfd_plugin_object_p.

Fixed.

> A perhaps more serious issue is that after your patch we'll call
> dlopen() and onload() on each lto input object file.  Before, we
> dlopen a plugin and call onload just once.

That is the price to pay for handling mixed (LLVM/GCC/native) archives.
If this is not desirable yet, I could perhaps add a variant of Rafael
commit 80549d04a2 back again.

Thanks for the review.

-- 
Markus


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]