[VDPAU] [PATCHv3] Add VDPAU_DRIVER_PATH support

[Emil Velikov]

On 10/03/14 16:48, Stephen Warren wrote:
> Emil Velikov wrote at Thursday, February 20, 2014 3:38 PM:
>> Allow the user to specify the location of the backend driver,
>> via the VDPAU_DRIVER_PATH environment variable. This allows
>> easier testing of VDPAU backends without the need to rebuild
>> libvdpau.
>>
>> Inspired by LIBGL_DRIVERS_PATH from mesa.
>
>> diff --git a/src/vdpau_wrapper.c b/src/vdpau_wrapper.c
>
>> +    /* Fallback to VDPAU_MODULEDIR when VDPAU_DRIVER_PATH is not set,
>> +     * or if we fail to create the driver path/dlopen the library. */
>> +    if (!_vdp_driver_dll &&
>> +        snprintf(vdpau_driver_lib, sizeof(vdpau_driver_lib), DRIVER_LIB_FORMAT,
>> +                 VDPAU_MODULEDIR, vdpau_driver) >=
>>               sizeof(vdpau_driver_lib)) {
>>           fprintf(stderr, "Failed to construct driver path: path too long\n");
>>           if (vdpau_driver_dri2) {
>> @@ -132,12 +149,14 @@ static VdpStatus _vdp_open_driver(
>>           return VDP_STATUS_NO_IMPLEMENTATION;
>>       }
>>
>> -    _vdp_driver_dll = dlopen(vdpau_driver_lib, RTLD_NOW | RTLD_GLOBAL);
>> +    if (!_vdp_driver_dll) {
>> +        _vdp_driver_dll = dlopen(vdpau_driver_lib, RTLD_NOW | RTLD_GLOBAL);
>> +    }
>
> That should really be the following:
>
> +    if (!_vdp_driver_dll) {
> ... existing sprint and dlopen code indented 1 level
> +    }
>
> With the patch as it is right now, that if condition is replicated in two
> places:
>
> if (!_vdp_driver_dll && sprint_ok) {
> }
> if (!_vdp_driver_dll) {
> }
>
> ... which makes the logic unclear.
>
While the logic may not be as straightforward I believe that things are 
correct as is. Unless I'm day dreaming that is :-)

Summary of control flow

if !setuid
     getenv && snprintf_ok && dlopen

if !_vdp_driver_dll      // dlopen was never ran or has failed
    && !snprintf_ok
       return NO_IMPL     // failed to construct the path, bailing out

if !_vdp_driver_dll      // dlopen never ran/failed
       // At this point vdpau_driver_lib is guaranteed to be valid
       dlopen             // try libvdpau_%s.so.1

if !_vdp_driver_dll      // last dlopen has failed
    snprintf && dlopen    // try libvdpau_%s.so

if vdpau_driver_dri2
...

If you prefer I can rework the conditionals like the following, although 
imho snprintf makes things "a bit" uglier and harder to follow but I do 
not mind too much either way.

if !setuid
    getenv && snprintf_ok && dlopen

if !_vdp_driver_dll {    // dlopen was never ran or has failed
    if !snprintf_ok
       return NO_IMPL     // failed to construct the path, bailing out
    else
       dlopen             // try libvdpau_%s.so.1
}

if !_vdp_driver_dll      // dlopen has failed
    snprintf && dlopen    // try libvdpau_%s.so


Cheers
-Emil