[PATCH xserver 15/15] dri3: correctly handle failed get_supported_modifiers requests

Emil Velikov emil.l.velikov at gmail.com
Fri Apr 6 11:18:53 UTC 2018


On 4 April 2018 at 19:51, Adam Jackson <ajax at nwnk.net> wrote:
> On Mon, 2018-04-02 at 16:41 +0100, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Currently depending on the code path hit, the helper will set some of
>> the output values and not others.
>>
>> It could also leak memory ;-)
>>
>> At the same time the caller was:
>>  - working around the broken behaviour - by initialising the variables
>>  - outright ignoring if the helper fails
>>
>> Fix all that by consistently setting the output variables and returning
>> a protocol error if we fail to get the data required.
>>
>> Bonus points: current code does not attribute if we cannot get the
>> modifiers info for certain format. A smaller format array is available,
>> yet the original length is stored.
>>
>> Fixes: cef12efc15c ("glamor: Implement GetSupportedModifiers")
>> Cc: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
>> Cc: Daniel Stone <daniels at collabora.com>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>> We could still return "success" to the user - I've opted for this
>> solution since we already bail on dixLookupWindow failure.
>
> This, combined with 10/15, means you will now throw BadImplementation
> when we used to succeed and simply report no modifiers. I can't think
> of a good reason to make that an error, it just makes the client's life
> harder if it has to distinguish error from reply.
>
I'm a bit split - on one hand any reasonable client should do error checking.
At least the Mesa one seems to do so.

On the other, the client would handle failed and 0 modifiers identically.

I'm fine either way. As soon as we get any other confirmation/comment
on this (or the rest of the series), I'll respin the lot.
There's a few conflicts with Dan's work.

Thanks
Emil


More information about the xorg-devel mailing list