This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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 v3 1/3] elf: Allow dlopen of filter object to work [BZ #16272]


> -----Original Message-----
> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 
> Since these are the only filter tests, I think we should also add some
> minimal one to check for invalid filter filter object (which does not
> provide the required symbol interfaces provided by the filtee).
> 
> I think also it is worth to extend the tests to cover for auxiliary
> filters as well (DT_AUXILIARY), since it intersects with DT_FILTERS.
> It should be similar to DT_FILTER tests, with the only difference
> that non existent auxiliary filters does not trigger a loader error
> (if the symbol is provided by the filter or another auxiliary filter).
> 
> The patch itself looks good, thanks for working on it.

Thanks for the feedback!

I'll rebase and take a look at resolving your comments on the 3 commits.
That may take a couple days :)

I think all your comments are clear enough, and I should be able to
deal with. Just on this one...

> > -  /* Sort the initializer list to take dependencies into account.
> The binary
> > -     itself will always be initialize last.  */
> > -  memcpy (l_initfini, map->l_searchlist.r_list,
> > -	  nlist * sizeof (struct link_map *));
...

> > +  if (i > 0)
> > +    {
> > +      /* Copy the binary into position 0.  */
> > +      memcpy (l_initfini, &map->l_searchlist.r_list[i],
> > +	      sizeof (struct link_map *));
> > +      /* Copy the filtees.  */
> > +      memcpy (&l_initfini[1], map->l_searchlist.r_list,
> > +	      i * sizeof (struct link_map *));
> > +      /* Copy the remainder.  */
> > +      memcpy (&l_initfini[i + 1], &map->l_searchlist.r_list[i + 1],
> > +	      (nlist - i - 1) * sizeof (struct link_map *));
> 
> Ok (although I not sure if is the correct idiom to use memcpy to
> copy array of pointers
> 
> 
> > +    }
> > +  else
> > +    memcpy (l_initfini, map->l_searchlist.r_list,
> > +	    nlist * sizeof (struct link_map *));
> > +

The original code was using a memcpy (presumably for speed), so I tried to
stick with that. I'm happy to change the new branch to use a loop to copy the
pointers - should I leave the original copy as a memcpy?



Dave.

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