[waffle] [PATCH 04/10] nacl: rework nacl_dl functions

[Tapani Pälli]

On 03/25/2015 04:07 PM, Emil Velikov wrote:
> On 25 March 2015 at 06:10, Tapani <tapani.palli at intel.com> wrote:
>> 2 small things below ..
>>
>>
>> On 03/24/2015 05:56 PM, Emil Velikov wrote:
>>>
> [...]
>>> +static bool
>>> +nacl_dl_check_enum(int32_t waffle_dl)
>>> +{
>>> +    switch (waffle_dl) {
>>> +        case WAFFLE_DL_OPENGL:
>>> +            wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
>>> +                         "NACL does not support OpenGL");
>>> +            return false;
>>> +        case WAFFLE_DL_OPENGL_ES1:
>>> +            wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
>>> +                         "NACL does not support OpenGL ES1");
>>> +            return false;
>>> +        case WAFFLE_DL_OPENGL_ES2:
>>> +            return true;
>>> +        case WAFFLE_DL_OPENGL_ES3:
>>> +            wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
>>> +                         "NACL does not support OpenGL ES3");
>>
>>
>> I don't see much value in in these messages, we only have 1 api that returns
>> true. If really wanted then just have default that prints the error using
>> wcore_enum_to_string() for the waffle_dl.
>>
> As mentioned above this is what waffle's cgl already does. If others
> are ok with nuking/reworking the similar hunk in cgl then I'm ok with
> it.

I think it's worth the trouble, if nothing else it is more readable.

>>> +            return false;
>>> +        default:
>>> +            assert(false);
>>> +            return false;
>>> +   }
>>> +}
>>> +
>>> +static bool
>>> +nacl_dl_open(struct nacl_platform *plat)
>>> +{
>>> +    plat->dl_gl = dlopen(NACL_GLES2_LIBRARY, RTLD_LAZY);
>>> +
>>> +    if (!plat->dl_gl) {
>>> +        wcore_errorf(WAFFLE_ERROR_UNKNOWN,
>>> +                     "dlopen(\"%s\") failed", NACL_GLES2_LIBRARY);
>>
>>
>> Would be cool to have the dlerror() message here.
>>
> Any objections if I keep this as a follow up change which updates both
> cgl and nacl ?

This is fine!

// Tapani