[PATCH xserver v2] Fix id in error when resource does not exist
Peter Harris
pharris at opentext.com
Thu Oct 6 18:06:21 UTC 2016
On 2016-10-06 1:07 PM, Adam Jackson wrote:
> On Thu, 2016-09-29 at 11:17 -0400, Peter Harris wrote:
>> Always set client->errorValue before returning an error.
>
> As sensible as this seems, I think it's wrong. I ran across something
> similar a few years ago:
>
> https://lists.freedesktop.org/archives/xorg-devel/2011-June/023462.html
>
> BadMatch errors are specified as not filling in errorValue, so...
>
>> @@ -1220,11 +1220,13 @@ dixLookupResourceByType(void **result, XID id, RESTYPE rtype,
>> if (res->id == id && res->type == rtype)
>> break;
>> }
>> + if (client) {
>> + client->errorValue = id;
>> + }
>> if (!res)
>> return resourceTypes[rtype & TypeMask].errorValue;
>>
>> if (client) {
>> - client->errorValue = id;
>> cid = XaceHook(XACE_RESOURCE_ACCESS, client, id, res->type,
>> res->value, RT_NONE, NULL, mode);
>> if (cid == BadValue)
>
> ... imagine ChangeGC specifying more than one of {tile, stipple, font,
> clipmask pixmap}. Whichever one is looked up last will be the
> errorValue thrown with the eventual BadMatch, which beyond being
> against the spec also means you might be misled about which resource is
> failing the match.
But that's already the case with the current code. When the lookup
succeeds, client->errorValue is set to whatever was looked up (by the
line removed in this patch). Yes, the next BadMatch is going to have
nonsense in a few bytes defined to be padding bytes, but that was
already the case.
This patch only moves the setting of client->errorValue above the
"Resource Not Found" return. So the error definitely[1] isn't BadMatch,
and we want the ID to be the thing that actually doesn't exist, instead
of the previous thing that does exist.
This patch only makes a difference[1] when the error value isn't BadMatch.
The thing that bit me was a BadGC with errorValue set to the drawable of
the offending request, which took some head-scratching to figure out.
Peter Harris
[1] Unless an extension calls SetResourceTypeErrorValue(resType,
BadMatch). Which would be crazy.
--
Open Text Connectivity Solutions Group
Peter Harris http://connectivity.opentext.com/
Research and Development Phone: +1 905 762 6001
pharris at opentext.com Toll Free: 1 877 359 4866
More information about the xorg-devel
mailing list