[PATCH xserver v2] Fix id in error when resource does not exist

[Adam Jackson]

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.

- ajax