[PATCH xserver 2/4] glx: Use vnd layer for dispatch (v3)

[Kyle Brenneman]

On 02/02/2018 12:15 PM, Adam Jackson wrote:
> The big change here is MakeCurrent and context tag tracking. We now
> delegate context tags entirely to the vnd layer, and simply store a
> pointer to the context state as the tag data. If a context is deleted
> while it's current, we allocate a fake ID for the context and move the
> context state there, so the tag data still points to a real context. As
> a result we can stop trying so hard to detach the client from contexts
> at disconnect time and just let resource destruction handle it.
>
> Since vnd handles all the MakeCurrent protocol now, our request handlers
> for it can just be return BadImplementation. We also remove a bunch of
> LEGAL_NEW_RESOURCE, because now by the time we're called vnd has already
> allocated its tracking resource on that XID.
>
> v2: Update to match v2 of the vnd import, and remove more redundant work
> like request length checks.
>
> v3: Add/remove the XID map from the vendor private thunk, not the
> backend. (Kyle Brenneman)
>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>   configure.ac                   |   2 +-
>   glx/createcontext.c            |   2 -
>   glx/glxcmds.c                  | 212 +++++--------------------
>   glx/glxcmdsswap.c              |  98 +-----------
>   glx/glxext.c                   | 348 +++++++++++++++++++++++++++++------------
>   glx/glxext.h                   |   4 +
>   glx/glxscreens.h               |   1 +
>   glx/glxserver.h                |   5 -
>   glx/xfont.c                    |   2 -
>   hw/kdrive/ephyr/ephyr.c        |   2 +-
>   hw/kdrive/ephyr/meson.build    |   1 +
>   hw/kdrive/src/kdrive.c         |   3 +
>   hw/vfb/InitOutput.c            |   2 +
>   hw/vfb/meson.build             |   3 +-
>   hw/xfree86/Makefile.am         |   5 +
>   hw/xfree86/common/xf86Init.c   |   2 +-
>   hw/xfree86/dixmods/glxmodule.c |   1 +
>   hw/xfree86/meson.build         |   1 +
>   hw/xquartz/darwin.c            |   4 +-
>   hw/xwayland/Makefile.am        |   1 +
>   hw/xwayland/meson.build        |   1 +
>   hw/xwayland/xwayland.c         |   2 +
>   include/glx_extinit.h          |   5 +-
>   23 files changed, 328 insertions(+), 379 deletions(-)
>

In __glXDisp_DestroyContext, doesn't it need to record the fake XID that 
it generates? If I'm reading it right, if the client deletes a current 
context and later unbinds it, then xorgGlxMakeCurrent will call 
FreeResourceByType with the original (already freed) XID, not with the 
fake one.

In xorgGlxThunkRequest, it needs to call glxServer.removeXIDMap to 
remove the XID for a GLXDestroyGLXPbufferSGIX request.

The handling for created XID's in xorgGlxThunkRequest looks correct. As 
a minor note, you could set the "resource" variable in the switch 
statement, rather than in a separate if/else block.

Also, I think the "if (!vendor)" block after the switch is dead code. 
Every branch in the switch assigns something to vendor and returns an 
error if it gets NULL.

-Kyle