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

[Emil Velikov]

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.

>>> +            return false;
>>> +
>>> +    switch (waffle_dl) {
>>> +        case WAFFLE_DL_OPENGL_ES2:
>>> +            nacl_name = nacl_prefix(name, "GLES2");
>>> +            break;
>>> +    }
>>> +
>>
>> Just drop the switch for now ?
>
> Can do. Thanks for the reviews Emil!
>
You're welcome :-)

-Emil