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

Tapani tapani.palli at intel.com
Thu Feb 5 05:12:47 PST 2015


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:
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> You might want to model dl handling similar to cgl_dl - both support
> only single waffle_dl.
> As such I won't pick on the diffs between the two.

Thanks, I'll take a look at this.

>> +// Construct a string that maps GL function to NaCl function
>> +// by concating given prefix and function name tail from 'src'.
>> +static char *
>> +nacl_prefix(const char *src, const char *prefix)
>> +{
>> +    if (strncmp(src, "gl", 2) != 0)
>> +        return NULL;
> wcore_error ?

Yep can add this, I was not sure if it is and error if symbol is not found.

>> +
>> +    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 (?)

>> +
>> +    char *dst = calloc(len, 1);
> Using wcore_calloc will set a nice waffle_error in case this fails :-)
Nice, will start to use that.

> [...]
>> @@ -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?

>> +            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!

> Cheers,
> Emil


// Tapani



More information about the waffle mailing list