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 phar...@opentext.com Toll Free: 1 877 359 4866 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel