Re: [PATCH v3 1/3] elf: Allow dlopen of filter object to work [BZ #16272]

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 15/01/2020 09:03, David Kilroy wrote:
>> -----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?

I don't have a strong preference, I think we can use the memcpy
code.