[VDPAU] [PATCH] Add VDPAU_DRIVER_PATH support

[Aaron Plattner]

On 05/18/14 01:33, Emil Velikov wrote:
> On 13/03/14 20:46, Aaron Plattner wrote:
>> On 03/13/2014 01:23 PM, Emil Velikov wrote:
>>> On 13/03/14 15:42, Stephen Warren wrote:
>>>> Emil Velikov wrote at Monday, March 10, 2014 2:13 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.
>>>>
>>>> Acked-by: Stephen Warren <swarren at nvidia.com>
>>>>
>>>> Just one question:
>>>>
>>>>> diff --git a/include/vdpau/vdpau_x11.h b/include/vdpau/vdpau_x11.h
>>>>
>>>>> +    if (geteuid() == getuid()) {
>>>>> +        /* don't allow setuid apps to use VDPAU_DRIVER_PATH */
>>>>> +        vdpau_driver_path = getenv("VDPAU_DRIVER_PATH");
>>>>> +        if (vdpau_driver_path &&
>>>>> +            snprintf(vdpau_driver_lib, sizeof(vdpau_driver_lib),
>>>>> +                     DRIVER_LIB_FORMAT, vdpau_driver_path, vdpau_driver) <
>>>>> +                sizeof(vdpau_driver_lib)) {
>>>>> +            _vdp_driver_dll = dlopen(vdpau_driver_lib, RTLD_NOW |
>>>>> RTLD_GLOBAL);
>>>>
>>>> If the snprintf() there fails, we just ignore it...
>>>>
>>>>> +    /* 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)
>>>>> +        if (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) {
>>>>> +                XFree(vdpau_driver_dri2);
>>>>> +                vdpau_driver_dri2 = NULL;
>>>>> +            }
>>>>> +            _VDP_ERROR_BREAKPOINT();
>>>>> +            return VDP_STATUS_NO_IMPLEMENTATION;
>>>>
>>>> ... whereas here, we actively return an error.
>>>>
>>>> Shouldn't those two paths be consistent; either each should return an error
>>>> (probably best?), or perhaps both should print a warning message, and then
>>>> the function should return an error at the end if (!_vdp_driver_dll)?
>>>>
>>> Interesting, I though I've covered this in an earlier email but I cannot
>>> seem to find it. Either way here it goes:
>>>
>>> I have deliberately tried not to change the original functionality - if
>>> for whatever reason we fail to load the getenv library we just fall back
>>> to the original methods. Think of unsuspecting user has
>>> VDPAU_DRIVER_PATH set in a way that snprint fails, while the original
>>> methods would succeed.
>>> With that said, a fprintf() would not hurt as a notification for the
>>> failed snprintf, but I would not want to add premature return here.
>>
>> I think that's fine.  I'll try to find some time to look at this patch soon,
>> and we can discuss changing the failure behavior in a separate thread.
>>
>> Thanks for working on this, Emil!
>>
> Humble ping.

Sorry, I fail miserably at time management.  I fixed up a missing curly 
brace and pushed this:

To git at github.com:/aaronp24/libvdpau.git
    186195b1a84f..ee9491a1216f  master -> master