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

[Chad Versace]

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.

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().


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/waffle/attachments/20150206/911a7865/attachment.sig>