[waffle] [PATCH 7/7] nacl: add implementation for waffle_dl_sym

Tapani tapani.palli at intel.com
Sat Feb 7 10:48:43 PST 2015


On 02/06/2015 09:44 PM, Chad Versace wrote:
> On 02/05/2015 09:05 AM, Emil Velikov wrote:
>> On 5 February 2015 at 13:12, Tapani <tapani.palli at intel.com> wrote:
>>> On 02/03/2015 07:09 PM, Emil Velikov wrote:
>>>> On 23 January 2015 at 07:59, Tapani Pälli <tapani.palli at intel.com> wrote:
>> ...
>>>>> +
>>>>> +    uint32_t len = strlen(src) + strlen(prefix);
>>>> So the function name changes from glHamSandwitch to GLES2HamSandwitch ?
>>>> In that case you'll need to subtract 2 from the len above.
>>> I believe only 1 because of the additional string terminator, but I consider
>>> above cleaner than playing the +/- game (?)
>>>
>> You're correct - I've completely forgot about \0, also I agree with
>> your argument.
>>
>> ...
>>>>> @@ -59,7 +97,28 @@ nacl_platform_dl_sym(struct wcore_platform *wc_self,
>>>>>                         int32_t waffle_dl,
>>>>>                         const char *name)
>>>>>    {
>>>>> -    return NULL;
>>>>> +    struct nacl_platform *self = nacl_platform(wc_self);
>>>>> +    char *nacl_name = NULL;
>>>>> +    void *func = NULL;
>>>>> +
>>>>> +    if (!self->gl_dl)
>>>>> +        if (!nacl_platform_dl_can_open(wc_self, waffle_dl))
>>>> One should not be doing this - it's the user's responsibility to call
>>>> dl_can_open prior to dl_sym.
>>>
>>> Documentation does not seem to indicate mandatory call to dl_can_open(), it
>>> just suggests that one can test if a dll can be opened this way. AFAICS one
>>> can just start to dlsym() the required functions?
>>>
>> I believe the lack of documentation is due to historical reasons.
>>
>> Imho it's a picky subject what is to be done, if one can do XX
>> context, but does not have (direct) library to dlsym the symbols from.
>> I believe I've annoyed Chad enough on the topic so I won't pick it up
>> again :-)
>>
>> I would personally opt to updating the documentation, as afaics none
>> of the current implementations calls dl_can_open within dl_sym.
>> Obviously the final decision is not mine to make, so this is just my 2c.
> The way that Waffle's Linux and Mac platforms work today, the user doesn't
> have to call waffle_dl_can_open(). When the user calls waffle_dl_sym(),
> Waffle internally calls dlopen() if the user hasn't called waffle_dl_sym()
> on that library before.
OK, thanks for this clarification.

> Tapani, when I reviewed it earlier, I
> misundertood how you were internally using nacl_platform_dl_can_open() inside
> nacl_platform_dl_sym(). But, now I think the way you're doing it is correct.
> I retract my earlier review comment.
>
> However, after looking more closely at the patch, it looks like
> nacl_platform_dl_can_open() re-opens the library with dlopen()
> each time the user calls waffle_dl_can_open().

oops yes, it's missing the check, I will fix this.

>
>
>
> _______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/waffle

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/waffle/attachments/20150207/31b590cc/attachment.html>


More information about the waffle mailing list