[RFC PATCH] gen_swap_check handles xproto and shape extensions

Christian Linhart chris at DemoRecorder.com
Wed Mar 11 07:22:07 PDT 2015


Hi Asalle,

Thank you for posting your changes as a patch.
(Next time, please do not sent it as attchment but in-message, because this is easier to handle for reviewers.
use git send-email for posting a patch)

Anyways, below is a quick review of your patch,
mostly focusing on stuff about patch-generation.

My comments are inserted below within the quoted version of your patch.

Please fix these things and regenerate a new version of your patch.

> >From 6f146415ee101d37d0127be91005ca37f6fbe928 Mon Sep 17 00:00:00 2001
> From: Asal Mirzaeva <asalle.kim at gmail.com>
> Date: Tue, 10 Mar 2015 14:47:00 +0200
> Subject: [PATCH] xorg-devel
>
> ---
>  Makefile.am             |   4 +-
>  Xext/Makefile.am        |   2 +-
>  Xext/shape.c            | 298 ++++++++++---------
>  configure.ac            |  78 +++--
>  proto/Makefile.am       |  27 +-
>  proto/gen_swap_check.py | 772 ++++++++++++++++++++++++++++--------------------
>  6 files changed, 667 insertions(+), 514 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 3bdfc8c..aac0a69 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,7 @@ SUBDIRS = \
>      dix  \
>      fb \
>      mi \
> +    proto \
>      Xext \
>      miext \
>      os \
> @@ -62,8 +63,7 @@ SUBDIRS = \
>      $(GLAMOR_DIR) \
>      config \
>      hw \
> -    test \
> -    proto
> +    test
>  
>  if XORG
>  aclocaldir = $(datadir)/aclocal
> diff --git a/Xext/Makefile.am b/Xext/Makefile.am
> index a9a4468..73ef643 100644
> --- a/Xext/Makefile.am
> +++ b/Xext/Makefile.am
> @@ -1,6 +1,6 @@
>  noinst_LTLIBRARIES = libXext.la libXextdpmsstubs.la
>  
> -AM_CFLAGS = $(DIX_CFLAGS)
> +AM_CFLAGS = $(DIX_CFLAGS) $(PROTO_CFLAGS)
>  
>  if XORG
>  sdk_HEADERS = xvdix.h xvmcext.h geext.h geint.h shmint.h syncsdk.h
> diff --git a/Xext/shape.c b/Xext/shape.c
> index bb479b1..00fc33d 100644
> --- a/Xext/shape.c
> +++ b/Xext/shape.c
> @@ -1026,7 +1026,7 @@ ProcShapeGetRectangles(ClientPtr client)
>  }
>  
>  static int
> -ProcShapeDispatch(ClientPtr client)
> +ProcShapeDispatch_Unchecked(ClientPtr client)
>  {
>      REQUEST(xReq);
>      switch (stuff->data) {
> @@ -1073,6 +1073,34 @@ ProcShapeDispatch(ClientPtr client)
>      }
>  }
>  
> +#include "swapcheck_shape.h"
> +
> +static int
> +ProcShapeDispatch(ClientPtr client)
> +{
> +    int check_result = xcb_shape_Check_dispatch(client);
> +    if(check_result == Success){
> +        return ProcShapeDispatch_Unchecked(client);
> +    }
> +    else
> +    {
> +        return check_result;
> +    }
> +}
> +
> +static int
> +SProcShapeDispatch(ClientPtr client)
> +{
> +    int swap_result = xcb_shape_SwapFromClient_dispatch(client);
> +    if(swap_result == Success){
> +        return ProcShapeDispatch_Unchecked(client);
> +    }
> +    else
> +    {
> +        return swap_result;
> +    }
> +}
> +
>  static void
>  SShapeNotifyEvent(xShapeNotifyEvent * from, xShapeNotifyEvent * to)
>  {
> @@ -1088,140 +1116,140 @@ SShapeNotifyEvent(xShapeNotifyEvent * from, xShapeNotifyEvent * to)
>      to->shaped = from->shaped;
>  }
>  
> -static int
> -SProcShapeQueryVersion(ClientPtr client)
> -{
> -    REQUEST(xShapeQueryVersionReq);
> -
> -    swaps(&stuff->length);
> -    return ProcShapeQueryVersion(client);
> -}
> -
> -static int
> -SProcShapeRectangles(ClientPtr client)
> -{
> -    REQUEST(xShapeRectanglesReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_AT_LEAST_SIZE(xShapeRectanglesReq);
> -    swapl(&stuff->dest);
> -    swaps(&stuff->xOff);
> -    swaps(&stuff->yOff);
> -    SwapRestS(stuff);
> -    return ProcShapeRectangles(client);
> -}
> -
> -static int
> -SProcShapeMask(ClientPtr client)
> -{
> -    REQUEST(xShapeMaskReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeMaskReq);
> -    swapl(&stuff->dest);
> -    swaps(&stuff->xOff);
> -    swaps(&stuff->yOff);
> -    swapl(&stuff->src);
> -    return ProcShapeMask(client);
> -}
> -
> -static int
> -SProcShapeCombine(ClientPtr client)
> -{
> -    REQUEST(xShapeCombineReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeCombineReq);
> -    swapl(&stuff->dest);
> -    swaps(&stuff->xOff);
> -    swaps(&stuff->yOff);
> -    swapl(&stuff->src);
> -    return ProcShapeCombine(client);
> -}
> -
> -static int
> -SProcShapeOffset(ClientPtr client)
> -{
> -    REQUEST(xShapeOffsetReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeOffsetReq);
> -    swapl(&stuff->dest);
> -    swaps(&stuff->xOff);
> -    swaps(&stuff->yOff);
> -    return ProcShapeOffset(client);
> -}
> -
> -static int
> -SProcShapeQueryExtents(ClientPtr client)
> -{
> -    REQUEST(xShapeQueryExtentsReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeQueryExtentsReq);
> -    swapl(&stuff->window);
> -    return ProcShapeQueryExtents(client);
> -}
> -
> -static int
> -SProcShapeSelectInput(ClientPtr client)
> -{
> -    REQUEST(xShapeSelectInputReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeSelectInputReq);
> -    swapl(&stuff->window);
> -    return ProcShapeSelectInput(client);
> -}
> -
> -static int
> -SProcShapeInputSelected(ClientPtr client)
> -{
> -    REQUEST(xShapeInputSelectedReq);
> -
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeInputSelectedReq);
> -    swapl(&stuff->window);
> -    return ProcShapeInputSelected(client);
> -}
> -
> -static int
> -SProcShapeGetRectangles(ClientPtr client)
> -{
> -    REQUEST(xShapeGetRectanglesReq);
> -    swaps(&stuff->length);
> -    REQUEST_SIZE_MATCH(xShapeGetRectanglesReq);
> -    swapl(&stuff->window);
> -    return ProcShapeGetRectangles(client);
> -}
> -
> -static int
> -SProcShapeDispatch(ClientPtr client)
> -{
> -    REQUEST(xReq);
> -    switch (stuff->data) {
> -    case X_ShapeQueryVersion:
> -        return SProcShapeQueryVersion(client);
> -    case X_ShapeRectangles:
> -        return SProcShapeRectangles(client);
> -    case X_ShapeMask:
> -        return SProcShapeMask(client);
> -    case X_ShapeCombine:
> -        return SProcShapeCombine(client);
> -    case X_ShapeOffset:
> -        return SProcShapeOffset(client);
> -    case X_ShapeQueryExtents:
> -        return SProcShapeQueryExtents(client);
> -    case X_ShapeSelectInput:
> -        return SProcShapeSelectInput(client);
> -    case X_ShapeInputSelected:
> -        return SProcShapeInputSelected(client);
> -    case X_ShapeGetRectangles:
> -        return SProcShapeGetRectangles(client);
> -    default:
> -        return BadRequest;
> -    }
> -}
> +//static int
> +//SProcShapeQueryVersion(ClientPtr client)
> +//{
> +    //REQUEST(xShapeQueryVersionReq);
> +
> +    //swaps(&stuff->length);
> +    //return ProcShapeQueryVersion(client);
> +//}
> +
> +//static int
> +//SProcShapeRectangles(ClientPtr client)
> +//{
> +    //REQUEST(xShapeRectanglesReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_AT_LEAST_SIZE(xShapeRectanglesReq);
> +    //swapl(&stuff->dest);
> +    //swaps(&stuff->xOff);
> +    //swaps(&stuff->yOff);
> +    //SwapRestS(stuff);
> +    //return ProcShapeRectangles(client);
> +//}
> +
> +//static int
> +//SProcShapeMask(ClientPtr client)
> +//{
> +    //REQUEST(xShapeMaskReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeMaskReq);
> +    //swapl(&stuff->dest);
> +    //swaps(&stuff->xOff);
> +    //swaps(&stuff->yOff);
> +    //swapl(&stuff->src);
> +    //return ProcShapeMask(client);
> +//}
> +
> +//static int
> +//SProcShapeCombine(ClientPtr client)
> +//{
> +    //REQUEST(xShapeCombineReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeCombineReq);
> +    //swapl(&stuff->dest);
> +    //swaps(&stuff->xOff);
> +    //swaps(&stuff->yOff);
> +    //swapl(&stuff->src);
> +    //return ProcShapeCombine(client);
> +//}
> +
> +//static int
> +//SProcShapeOffset(ClientPtr client)
> +//{
> +    //REQUEST(xShapeOffsetReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeOffsetReq);
> +    //swapl(&stuff->dest);
> +    //swaps(&stuff->xOff);
> +    //swaps(&stuff->yOff);
> +    //return ProcShapeOffset(client);
> +//}
> +
> +//static int
> +//SProcShapeQueryExtents(ClientPtr client)
> +//{
> +    //REQUEST(xShapeQueryExtentsReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeQueryExtentsReq);
> +    //swapl(&stuff->window);
> +    //return ProcShapeQueryExtents(client);
> +//}
> +
> +//static int
> +//SProcShapeSelectInput(ClientPtr client)
> +//{
> +    //REQUEST(xShapeSelectInputReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeSelectInputReq);
> +    //swapl(&stuff->window);
> +    //return ProcShapeSelectInput(client);
> +//}
> +
> +//static int
> +//SProcShapeInputSelected(ClientPtr client)
> +//{
> +    //REQUEST(xShapeInputSelectedReq);
> +
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeInputSelectedReq);
> +    //swapl(&stuff->window);
> +    //return ProcShapeInputSelected(client);
> +//}
> +
> +//static int
> +//SProcShapeGetRectangles(ClientPtr client)
> +//{
> +    //REQUEST(xShapeGetRectanglesReq);
> +    //swaps(&stuff->length);
> +    //REQUEST_SIZE_MATCH(xShapeGetRectanglesReq);
> +    //swapl(&stuff->window);
> +    //return ProcShapeGetRectangles(client);
> +//}
> +
> +//static int
> +//SProcShapeDispatch(ClientPtr client)
> +//{
> +    //REQUEST(xReq);
> +    //switch (stuff->data) {
> +    //case X_ShapeQueryVersion:
> +        //return SProcShapeQueryVersion(client);
> +    //case X_ShapeRectangles:
> +        //return SProcShapeRectangles(client);
> +    //case X_ShapeMask:
> +        //return SProcShapeMask(client);
> +    //case X_ShapeCombine:
> +        //return SProcShapeCombine(client);
> +    //case X_ShapeOffset:
> +        //return SProcShapeOffset(client);
> +    //case X_ShapeQueryExtents:
> +        //return SProcShapeQueryExtents(client);
> +    //case X_ShapeSelectInput:
> +        //return SProcShapeSelectInput(client);
> +    //case X_ShapeInputSelected:
> +        //return SProcShapeInputSelected(client);
> +    //case X_ShapeGetRectangles:
> +        //return SProcShapeGetRectangles(client);
> +    //default:
> +        //return BadRequest;
> +    //}
> +//}

Please remove these commented-out things.
There is no need to keep old code in this way.
The old is accessible anyways in the git-history.

>  
>  void
>  ShapeExtensionInit(void)
> diff --git a/configure.ac b/configure.ac
> index ef9f0bc..92d3a62 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -35,9 +35,6 @@ AM_INIT_AUTOMAKE([foreign dist-bzip2])
>  AC_USE_SYSTEM_EXTENSIONS
>  AM_PATH_PYTHON([2.7])
>  
> -
> -
> -
Removing the empty lines is unrelated to your changes.
Please omit this in your patch (i.e re-include these lines in your private repo and regenerate your patch).
>  # Require xorg-macros minimum of 1.14 for XORG_COMPILER_BRAND in XORG_DEFAULT_OPTIONS
>  m4_ifndef([XORG_MACROS_VERSION],
>            [m4_fatal([must install xorg-macros 1.14 or later before running autoconf/autogen])])
> @@ -98,7 +95,7 @@ if  test "x$GCC" = xyes ; then
>  fi
>  
>  dnl Check for dtrace program (needed to build Xserver dtrace probes)
> -dnl Also checks for <sys/sdt.h>, since some Linux distros have an
> +dnl Also checks for <sys/sdt.h>, since some Linux distros have an
>  dnl ISDN trace program named dtrace
>  AC_ARG_WITH(dtrace, AS_HELP_STRING([--with-dtrace=PATH],
>           [Enable dtrace probes (default: enabled if dtrace found)]),
> @@ -118,7 +115,7 @@ if test "x$WDTRACE" = "xyes" -o "x$WDTRACE" = "xauto" ; then
>      fi
>  fi
>  if test "x$WDTRACE" != "xno" ; then
> -  AC_DEFINE(XSERVER_DTRACE, 1,
This removes trailing space of code that has nothing to do with your changes.
If you want to do that, please make an extra patch for these things.
You can also just omit these changes.
(i.e. include the old trailing whitespaces in your private repo, and regenerating the patch.)

This happens multiple times here.

> +  AC_DEFINE(XSERVER_DTRACE, 1,
>        [Define to 1 if the DTrace Xserver provider probes should be built in.])
>  
>  # Solaris/OpenSolaris require dtrace -G to build dtrace probe information into
> @@ -192,7 +189,7 @@ b = bswap16(a);
>          if test "$SYS_ENDIAN_BSWAP" = "yes" ; then
>          USE_SYS_ENDIAN_H=yes
>          BSWAP=bswap
> -    else    
> +    else
removal of old trailing space that has nothing to do with your changes.
>              if test "$SYS_ENDIAN__SWAP" = "yes" ; then
>              USE_SYS_ENDIAN_H=yes
>              BSWAP=__swap
> @@ -202,13 +199,13 @@ b = bswap16(a);
>      fi
>  
>      if test "$USE_SYS_ENDIAN_H" = "yes" ; then
> -        AC_DEFINE([USE_SYS_ENDIAN_H], 1,
> +        AC_DEFINE([USE_SYS_ENDIAN_H], 1,
removal of old trailing space that has nothing to do with your changes.
>          [Define to use byteswap macros from <sys/endian.h>])
> -        AC_DEFINE_UNQUOTED([bswap_16], ${BSWAP}16,
> +        AC_DEFINE_UNQUOTED([bswap_16], ${BSWAP}16,
removal of old trailing space that has nothing to do with your changes.
>              [Define to 16-bit byteswap macro])
> -        AC_DEFINE_UNQUOTED([bswap_32], ${BSWAP}32,
> +        AC_DEFINE_UNQUOTED([bswap_32], ${BSWAP}32,
removal of old trailing space that has nothing to do with your changes.
>              [Define to 32-bit byteswap macro])
> -        AC_DEFINE_UNQUOTED([bswap_64], ${BSWAP}64,
> +        AC_DEFINE_UNQUOTED([bswap_64], ${BSWAP}64,
removal of old trailing space that has nothing to do with your changes.
>              [Define to 64-bit byteswap macro])
>      fi
>  fi
> @@ -256,7 +253,7 @@ AC_CACHE_CHECK([for SYSV IPC],
>  #include <sys/shm.h>
>  #include <sys/stat.h>
>  ]],[[
> -{
> +{
removal of old trailing space that has nothing to do with your changes.
>      int id;
>      id = shmget(IPC_PRIVATE, 512, S_IRUSR | S_IWUSR);
>      if (id < 0) return -1;
> @@ -268,12 +265,12 @@ if test "x$ac_cv_sysv_ipc" = xyes; then
>      AC_DEFINE(HAVE_SYSV_IPC, 1, [Define to 1 if SYSV IPC is available])
>  fi
>  
> -dnl OpenBSD /dev/xf86 aperture driver
> +dnl OpenBSD /dev/xf86 aperture driver
removal of old trailing space that has nothing to do with your changes.
>  if test -c /dev/xf86 ; then
>      AC_DEFINE(HAS_APERTURE_DRV, 1, [System has /dev/xf86 aperture driver])
>  fi
>  
> -dnl BSD APM support
> +dnl BSD APM support
>  AC_CHECK_HEADER([machine/apmvar.h],[
>      AC_CHECK_HEADER([sys/event.h],
>          ac_cv_BSD_KQUEUE_APM=yes,
> @@ -281,7 +278,7 @@ AC_CHECK_HEADER([machine/apmvar.h],[
>  
>  AM_CONDITIONAL(BSD_APM, [test "x$ac_cv_BSD_APM" = xyes])
>  AM_CONDITIONAL(BSD_KQUEUE_APM, [test "x$ac_cv_BSD_KQUEUE_APM" = xyes])
> -    
> +
removal of old trailing space that has nothing to do with your changes.
>  dnl glibc backtrace support check
>  AC_CHECK_HEADER([execinfo.h],[
>      AC_CHECK_LIB(c, backtrace, [
> @@ -319,7 +316,7 @@ case $host_cpu in
>          *netbsd*)    AC_DEFINE(USE_I386_IOPL)
>                  SYS_LIBS=-li386
>                  ;;
> -        *openbsd*)    AC_DEFINE(USE_I386_IOPL)
> +        *openbsd*)    AC_DEFINE(USE_I386_IOPL)
removal of old trailing space that has nothing to do with your changes.
>                  SYS_LIBS=-li386
>                  ;;
>      esac
> @@ -580,6 +577,7 @@ AC_ARG_WITH(khronos-spec-dir, AS_HELP_STRING([--with-khronos-spec-dir=PATH], [Pa
>                  [KHRONOS_SPEC_DIR=auto])
>  
>  dnl Extensions.
> +AC_ARG_ENABLE(proto,          AS_HELP_STRING([--disable-composite], [Build Proto (default: enabled)]), [PROTO=$enableval], [PROTO=yes])
>  AC_ARG_ENABLE(composite,      AS_HELP_STRING([--disable-composite], [Build Composite extension (default: enabled)]), [COMPOSITE=$enableval], [COMPOSITE=yes])
>  AC_ARG_ENABLE(mitshm,         AS_HELP_STRING([--disable-mitshm], [Build SHM extension (default: auto)]), [MITSHM=$enableval], [MITSHM=auto])
>  AC_ARG_ENABLE(xres,           AS_HELP_STRING([--disable-xres], [Build XRes extension (default: enabled)]), [RES=$enableval], [RES=yes])
> @@ -646,7 +644,7 @@ AC_ARG_ENABLE(xshmfence,      AS_HELP_STRING([--disable-xshmfence], [Disable xsh
>  
>  dnl chown/chmod to be setuid root as part of build
>  dnl Replaces InstallXserverSetUID in imake
> -AC_ARG_ENABLE(install-setuid,
> +AC_ARG_ENABLE(install-setuid,
>      AS_HELP_STRING([--enable-install-setuid],
>         [Install Xorg server as owned by root with setuid bit (default: auto)]),
>      [SETUID=$enableval], [SETUID=auto])
> @@ -732,7 +730,7 @@ case $host_os in
>                                 [xorg_cv_Carbon_framework=yes],
>                                 [xorg_cv_Carbon_framework=no])
>                      LDFLAGS=$save_LDFLAGS])
> -                
> +
removal of old trailing space that has nothing to do with your changes.
>              if test "X$xorg_cv_Carbon_framework" = Xyes; then
>                  XQUARTZ=yes
>              else
> @@ -1040,6 +1038,13 @@ if test "x$COMPOSITE" = xyes; then
>      COMPOSITE_INC='-I$(top_srcdir)/composite'
>  fi
>  
> +AM_CONDITIONAL(PROTO, [test "x$PROTO" = xyes])
> +if test "x$PROTO" = xyes; then
> +    AC_DEFINE(PROTO, 1, [Support Proto])
> +    PROTO_LIB='$(top_builddir)/proto/libproto.la'
> +    PROTO_INC='-I$(top_srcdir)/proto/generated'
> +fi
> +
>  if test "x$MITSHM" = xauto; then
>      MITSHM="$ac_cv_sysv_ipc"
>  fi
> @@ -1780,31 +1785,24 @@ AC_EGREP_CPP([I_AM_SVR4],[
>  AC_DEFINE([SVR4],1,[Define to 1 on systems derived from System V Release 4])
>  AC_MSG_RESULT([yes])], AC_MSG_RESULT([no]))
>  
> -XSERVER_CFLAGS="$XSERVER_CFLAGS $CORE_INCS $XEXT_INC $COMPOSITE_INC $DAMAGE_INC $FIXES_INC $XI_INC $MI_INC $MIEXT_SYNC_INC $MIEXT_SHADOW_INC $MIEXT_LAYER_INC $MIEXT_DAMAGE_INC $RENDER_INC $RANDR_INC $FB_INC $DBE_INC $PRESENT_INC"
> +XSERVER_CFLAGS="$XSERVER_CFLAGS $CORE_INCS $XEXT_INC $COMPOSITE_INC $DAMAGE_INC $FIXES_INC $XI_INC $MI_INC $MIEXT_SYNC_INC $MIEXT_SHADOW_INC $MIEXT_LAYER_INC $MIEXT_DAMAGE_INC $RENDER_INC $RANDR_INC $FB_INC $DBE_INC $PRESENT_INC $PROTO_INC"
>  
>  dnl ---------------------------------------------------------------------------
>  dnl proto section.
>  dnl ---------------------------------------------------------------------------
> -
>  # Find the xcb-proto protocol descriptions
> -AC_MSG_CHECKING(XCBPROTO_XCBINCLUDEDIR)
> +
> +PKG_CHECK_MODULES([XCBPROTO], [xcb-proto >= 1.11])
> +
>  XCBPROTO_XCBINCLUDEDIR=`$PKG_CONFIG --variable=xcbincludedir xcb-proto`
> -AC_MSG_RESULT($XCBPROTO_XCBINCLUDEDIR)
>  AC_SUBST(XCBPROTO_XCBINCLUDEDIR)
>  
> -# Find the xcb-proto version
> -XCBPROTO_VERSION=`$PKG_CONFIG --modversion xcb-proto`
> -AC_SUBST(XCBPROTO_VERSION)
> -
>  # Find the xcbgen Python package
> -AC_MSG_CHECKING(XCBPROTO_XCBPYTHONDIR)
>  XCBPROTO_XCBPYTHONDIR=`$PKG_CONFIG --variable=pythondir xcb-proto`
> -AC_MSG_RESULT($XCBPROTO_XCBPYTHONDIR)
>  AC_SUBST(XCBPROTO_XCBPYTHONDIR)
>  
>  # CFLAGS
> -libxcb_inc=`$PKG_CONFIG --cflags xcb`
> -PROTO_CFLAGS="$XSERVER_CFLAGS $libxcb_inc"
> +PROTO_CFLAGS="$XSERVER_CFLAGS $XCBPROTO_CFLAGS"
>  AC_SUBST([PROTO_CFLAGS])
>  
>  dnl ---------------------------------------------------------------------------
> @@ -1818,7 +1816,7 @@ AC_MSG_RESULT([$XVFB])
>  AM_CONDITIONAL(XVFB, [test "x$XVFB" = xyes])
>  
>  if test "x$XVFB" = xyes; then
> -    XVFB_LIBS="$FB_LIB $FIXES_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB"
> +    XVFB_LIBS="$FB_LIB $FIXES_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $PROTO_LIB"
>      XVFB_SYS_LIBS="$XVFBMODULES_LIBS $GLX_SYS_LIBS"
>      AC_SUBST([XVFB_LIBS])
>      AC_SUBST([XVFB_SYS_LIBS])
> @@ -1839,7 +1837,7 @@ if test "x$XNEST" = xyes; then
>      if test "x$have_xnest" = xno; then
>          AC_MSG_ERROR([Xnest build explicitly requested, but required modules not found.])
>      fi
> -    XNEST_LIBS="$FB_LIB $FIXES_LIB $MI_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB  $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB"
> +    XNEST_LIBS="$FB_LIB $FIXES_LIB $MI_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB  $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB $PROTO_LIB"
>      XNEST_SYS_LIBS="$XNESTMODULES_LIBS $GLX_SYS_LIBS"
>      AC_SUBST([XNEST_LIBS])
>      AC_SUBST([XNEST_SYS_LIBS])
> @@ -1864,7 +1862,7 @@ if test "x$XORG" = xyes; then
>      XORG_OSINCS='-I$(top_srcdir)/hw/xfree86/os-support -I$(top_srcdir)/hw/xfree86/os-support/bus -I$(top_srcdir)/os'
>      XORG_INCS="$XORG_DDXINCS $XORG_OSINCS"
>      XORG_CFLAGS="$XORGSERVER_CFLAGS -DHAVE_XORG_CONFIG_H"
> -    XORG_LIBS="$COMPOSITE_LIB $FIXES_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $RANDR_LIB $RENDER_LIB $DAMAGE_LIB $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $XI_LIB $XKB_LIB"
> +    XORG_LIBS="$COMPOSITE_LIB $FIXES_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $RANDR_LIB $RENDER_LIB $DAMAGE_LIB $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $XI_LIB $XKB_LIB $PROTO_LIB"
>  
>      dnl ==================================================================
>      dnl symbol visibility
> @@ -2000,12 +1998,12 @@ if test "x$XORG" = xyes; then
>              AC_MSG_ERROR([This release no longer supports Solaris versions older than Solaris 8.])
>          fi
>          AC_CHECK_DECL([_LP64], [SOLARIS_64="yes"], [SOLARIS_64="no"])
> -            
> +
removal of old trailing space that has nothing to do with your changes.
>          case $host_cpu in
> -          sparc*)    
> +          sparc*)
removal of old trailing space that has nothing to do with your changes.
>              SOLARIS_INOUT_ARCH="sparcv8plus"
>              ;;
> -          i*86)    
> +          i*86)
removal of old trailing space that has nothing to do with your changes.
>              if test x$SOLARIS_64 = xyes ; then
>                  SOLARIS_INOUT_ARCH="amd64"
>              else
> @@ -2415,16 +2413,16 @@ if test "$KDRIVE" = yes; then
>      # Xephyr needs nanosleep() which is in librt on Solaris
>      AC_CHECK_FUNC([nanosleep], [],
>          AC_CHECK_LIB([rt], [nanosleep], XEPHYR_LIBS="$XEPHYR_LIBS -lrt"))
> -    
> +
removal of old trailing space that has nothing to do with your changes.
>      # damage shadow extension glx (NOTYET) fb mi
>      KDRIVE_INC='-I$(top_srcdir)/hw/kdrive/src'
>      KDRIVE_PURE_INCS="$KDRIVE_INC $MIEXT_SYNC_INC $MIEXT_DAMAGE_INC $MIEXT_SHADOW_INC $XEXT_INC $FB_INC $MI_INC"
>      KDRIVE_OS_INC='-I$(top_srcdir)/hw/kdrive/linux'
> -    KDRIVE_INCS="$KDRIVE_PURE_INCS $KDRIVE_OS_INC"
> -    
> +    KDRIVE_INCS="$KDRIVE_PURE_INCS $KDRIVE_OS_INC $PROTO_INC"
> +
>      KDRIVE_CFLAGS="$XSERVER_CFLAGS -DHAVE_KDRIVE_CONFIG_H $TSLIB_CFLAGS"
>  
> -    KDRIVE_PURE_LIBS="$FB_LIB $MI_LIB $FIXES_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $OS_LIB"
> +    KDRIVE_PURE_LIBS="$FB_LIB $MI_LIB $FIXES_LIB $XEXT_LIB $DBE_LIB $RECORD_LIB $GLX_LIBS $RANDR_LIB $RENDER_LIB $DAMAGE_LIB $DRI3_LIB $PRESENT_LIB $MIEXT_SYNC_LIB $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $OS_LIB $PROTO_LIB"
>      KDRIVE_LIB='$(top_builddir)/hw/kdrive/src/libkdrive.la'
>      case $host_os in
>      *linux*)
> @@ -2503,7 +2501,7 @@ fi
>  
>  
>  dnl and the rest of these are generic, so they're in config.h
> -dnl
> +dnl
>  dnl though, thanks to the passing of some significant amount of time, the
>  dnl above is probably a complete fallacy, and you should not rely on it.
>  dnl but this is still actually better than imake, honest. -daniels
> diff --git a/proto/Makefile.am b/proto/Makefile.am
> index a2cbb23..34af921 100644
> --- a/proto/Makefile.am
> +++ b/proto/Makefile.am
> @@ -1,26 +1,21 @@
>  noinst_LTLIBRARIES = libproto.la
> -
> +
removal of old trailing space that has nothing to do with your changes.
>  AM_CFLAGS = $(PROTO_CFLAGS)
>  
> -EXTSOURCES =    \
> -    gen/swapcheck_xproto.c \
> -    gen/swapcheck_render.c
> -    gen/swapcheck_shape.c \
> +prefx=swapcheck_
> +PROTO_GENERATEDDIR=generated
>  
> +extension_sources =    \
> +    ${PROTO_GENERATEDDIR}/${prefx}xproto.c \
> +    ${PROTO_GENERATEDDIR}/${prefx}shape.c
>  
> -libproto_la_SOURCES = $(EXTSOURCES)
> +libproto_la_SOURCES = $(extension_sources)
>  
> -$(EXTSOURCES): $(XCBPROTO_XCBINCLUDEDIR)/$(@:.c=.xml) $(srcdir)/gen_swap_check.py
> +sources = $(subst ${PROTO_GENERATEDDIR}/${prefx}, ,$(@))
>  
> -    $(AM_V_GEN)$(PYTHON) $(srcdir)/gen_swap_check.py \
> -        -p $(XCBPROTO_XCBPYTHONDIR) \
> -        $(XCBPROTO_XCBINCLUDEDIR)/xproto.xml
> -        
> -    $(AM_V_GEN)$(PYTHON) $(srcdir)/gen_swap_check.py \
> -        -p $(XCBPROTO_XCBPYTHONDIR) \
> -        $(XCBPROTO_XCBINCLUDEDIR)/shape.xml
> +$(extension_sources): $(XCBPROTO_XCBINCLUDEDIR)/$(sources:.c=.xml) $(srcdir)/gen_swap_check.py
>  
>      $(AM_V_GEN)$(PYTHON) $(srcdir)/gen_swap_check.py \
>          -p $(XCBPROTO_XCBPYTHONDIR) \
> -        $(XCBPROTO_XCBINCLUDEDIR)/render.xml
> -
> +        -d ${PROTO_GENERATEDDIR} \
> +        $(XCBPROTO_XCBINCLUDEDIR)/$(sources:.c=.xml)
> diff --git a/proto/gen_swap_check.py b/proto/gen_swap_check.py
> index ae6675a..f0ca86e 100644
> --- a/proto/gen_swap_check.py
> +++ b/proto/gen_swap_check.py
There is something wrong with the base for your patch.
The upstream X-Server does not contain proto/gen_swap_check.py,
so your patch should list the whole files with only "+" lines.

But your patch contains changes relative to some internal version of your code.

Please fix this. You can ask me by personal mail if unsure how to fix this.

> @@ -5,141 +5,167 @@ import sys
>  import errno
>  import re
>  
> +def _i():
> +    '''
> +    Indent function
> +
> +    Returns joined _indent to be concatenated with code string in _c()
> +    '''
> +    return ''.join(_indent)
>  
> -#Helper functions and definitions copy-pasted from c_client.py, to replace with xcbutils.py
> +# Helping functions and definitions copy-pasted from c_client.py
>  
>  def _h(fmt, *args):
> -    '''
> -    Writes the given line to the header file.
> -    '''
> -    _hlines[_hlevel].append(fmt % args)
> +    '''
> +    Writes the given line to the header file.
> +    '''
> +    _hlines[_hlevel].append(fmt % args)
>  
>  def _c(fmt, *args):
> -    '''
> -    Writes the given line to the source file.
> -    '''
> -    _clines[_clevel].append(fmt % args)
> +    '''
> +    Writes the given line to the source file.
> +    '''
> +    _clines[_clevel].append(fmt % args)
>  
>  def _hc(fmt, *args):
> -    '''
> -    Writes the given line to both the header and source files.
> -    '''
> -    _h(fmt, *args)
> -    _c(fmt, *args)
> -
> -def _c_wr_stringlist(indent, strlist):
> -    '''
> -    Writes the given list of strings to the source file.
> -    Each line is prepended by the indent string
> -    '''
> -    for str in strlist:
> -        _c("%s%s", indent, str)
> +    '''
> +    Writes the given line to both the header and source files.
> +    '''
> +    _h(fmt, *args)
> +    _c(fmt, *args)
> +
> +def _code(fmt, *args):
> +    global _codelines
> +    _codelines.append(fmt % args)
> +
> +def output_code():
> +    global _codelines
> +    for line in _codelines:
> +        _c("%s", line)
> +    _codelines = []
>  
>  # Some hacks to make the API more readable and to keep backwards compability
>  _cname_special_cases = {'DECnet':'decnet'}
>  _extension_special_cases = ['XPrint', 'XCMisc', 'BigRequests']
> +
> +# The regex matches three types of strings:
> +#    1. Those starting with one and only one upper-case letter or a digit
> +#        and proceeding with lower-case letters, no upper-case letters or
> +#        digits are allowed except for the first letter
> +#    2. Those staring and proceeding with upper-case letters or digits and
> +#        containing no lower-case letters at all
> +#    3. Those starting and proceeding with lower-case letters and containing
> +#        no upper-case letters or digits at all
>  _cname_re = re.compile('([A-Z0-9][a-z]+|[A-Z0-9]+(?![a-z])|[a-z]+)')
>  
>  _hlines = []
>  _hlevel = 0
>  _clines = []
>  _clevel = 0
> +_codelines = []
> +
>  _ns = None
>  _filename = ''
> -requests = {}
> +_requests = {}
> +_indent = []
> +
> +def _increase_indent():
> +    global _indent
> +    _indent.append('    ')
> +
> +def _decrease_indent():
> +    global _indent
> +    if _indent: # if not empty
> +        _indent.pop()
>  
>  # XXX See if this level thing is really necessary.
>  def _h_setlevel(idx):
> -    '''
> -    Changes the array that header lines are written to.
> -    Supports writing different sections of the header file.
> -    '''
> -    global _hlevel
> -    while len(_hlines) <= idx:
> -        _hlines.append([])
> -    _hlevel = idx
> +    '''
> +    Changes the array that header lines are written to.
> +    Supports writing different sections of the header file.
> +    '''
> +    global _hlevel
> +    while len(_hlines) <= idx:
> +        _hlines.append([])
> +    _hlevel = idx
>  
>  def _c_setlevel(idx):
> -    '''
> -    Changes the array that source lines are written to.
> -    Supports writing to different sections of the source file.
> -    '''
> -    global _clevel
> -    while len(_clines) <= idx:
> -        _clines.append([])
> -    _clevel = idx
> -    
> +    '''
> +    Changes the array that source lines are written to.
> +    Supports writing to different sections of the source file.
> +    '''
> +    global _clevel
> +    while len(_clines) <= idx:
> +        _clines.append([])
> +    _clevel = idx
> +
>  def _n_item(str):
> -    '''
> -    Does C-name conversion on a single string fragment.
> -    Uses a regexp with some hard-coded special cases.
> -    '''
> -    if str in _cname_special_cases:
> -        return _cname_special_cases[str]
> -    else:
> -        split = _cname_re.finditer(str)
> -        name_parts = [match.group(0) for match in split]
> -        return '_'.join(name_parts)
> +    '''
> +    Does C-name conversion on a single string fragment.
> +    Uses a regexp with some hard-coded special cases.
> +    '''
> +    if str in _cname_special_cases:
> +        return _cname_special_cases[str]
> +    else:
> +        split = _cname_re.finditer(str)
> +        name_parts = [match.group(0) for match in split]
> +        return '_'.join(name_parts)
>  
>  def _ext(str):
> -    '''
> -    Does C-name conversion on an extension name.
> -    Has some additional special cases on top of _n_item.
> -    '''
> -    if str in _extension_special_cases:
> -        return _n_item(str).lower()
> -    else:
> -        return str.lower()
> +    '''
> +    Does C-name conversion on an extension name.
> +    Has some additional special cases on top of _n_item.
> +    '''
> +    if str in _extension_special_cases:
> +        return _n_item(str).lower()
> +    else:
> +        return str.lower()
>  
>  def _n(list):
> -    '''
> -    Does C-name conversion on a tuple of strings.
> -    Different behavior depending on length of tuple, extension/not extension, etc.
> -    Basically C-name converts the individual pieces, then joins with underscores.
> -    '''
> -    if len(list) == 1:
> -        parts = list
> -    elif len(list) == 2:
> -        parts = [list[0], _n_item(list[1])]
> -    elif _ns.is_ext:
> -        parts = [list[0], _ext(list[1])] + [_n_item(i) for i in list[2:]]
> -    else:
> -        parts = [list[0]] + [_n_item(i) for i in list[1:]]
> -    return '_'.join(parts).lower()
> -
> -def _t(list):
> -    '''
> -    Does C-name conversion on a tuple of strings representing a type.
> -    Same as _n but adds a "_t" on the end.
> -    '''
> -    if len(list) == 1:
> -        parts = list
> -    elif len(list) == 2:
> -        parts = [list[0], _n_item(list[1]), 't']
> -    elif _ns.is_ext:
> -        parts = [list[0], _ext(list[1])] + [_n_item(i) for i in list[2:]] + ['t']
> -    else:
> -        parts = [list[0]] + [_n_item(i) for i in list[1:]] + ['t']
> -    return '_'.join(parts).lower()
> -
> -#to replace part ends here
> -
> -
> -#                    Fuctions required by output
> +    '''
> +    Does C-name conversion on a tuple of strings.
> +    Different behavior depending on length of tuple, extension/not extension, etc.
> +    Basically C-name converts the individual pieces, then joins with underscores.
> +    '''
> +    if len(list) == 1:
> +        parts = list
> +    elif len(list) == 2:
> +        parts = [list[0], _n_item(list[1])]
> +    elif _ns.is_ext:
> +        parts = [list[0], _ext(list[1])] + [_n_item(i) for i in list[2:]]
> +    else:
> +        parts = [list[0]] + [_n_item(i) for i in list[1:]]
> +    return '_'.join(parts).lower()
> +
> +#            Copy-pasted from c_client.py code ends here
> +
> +# Check for the argument that specifies path to the xcbgen python package.
> +try:
> +    opts, args = getopt.getopt(sys.argv[1:], 'd:p:')
> +except getopt.GetoptError as err:
> +    print(err)
> +    print('Usage: gen_swap_check.py -d generated_dir [-p path] file.xml')
> +    sys.exit(1)
> +
> +for (opt, arg) in opts:
> +    if opt == '-d':
> +        gendir = arg
> +    if opt == '-p':
> +        sys.path.insert(1, arg)
>  
>  def c_open(self):
>      '''
> -    Exported function that handles module open.
> -    Opens the files and writes out the auto-generated comment,
> -    header file includes, etc.
> -    '''
> +    Exported function that handles module open.
> +    Opens the files and writes out the auto-generated comment,
> +    header file includes, etc.
> +    '''
>      global _ns
>      _ns = self.namespace
>      _ns.c_ext_global_name = _n(_ns.prefix + ('id',))
> -    
> +
>      global _filename
>      _filename = ''.join(('swapcheck_',_ns.header))
> -    
> +
>      _h_setlevel(0)
>      _c_setlevel(0)
>  
> @@ -158,31 +184,22 @@ def c_open(self):
>      _h('#ifndef __%s_H', _ns.header.upper())
>      _h('#define __%s_H', _ns.header.upper())
>      _h('')
> -    
> +
> +    # swapcheck_xproto.h is included in all the others' extensions header files, so
> +    # it's very convenient to include the common libs into this header
>      if _ns.header == 'xproto':
> -        _h('#include "xcb/xcb.h"')
>          _h('#include "xorg/misc.h"')
>          _h('#include "X11/X.h"')
>          _h('#include "X11/Xproto.h"')
>      else:
> -        pass
> -        
> -    _c('//#ifdef HAVE_CONFIG_H')
> -    _c('//#include "config.h"')
> -    _c('//#endif')
> -    
> -    if _ns.is_ext:
> -        for (n, h) in self.direct_imports:
> -            _hc('#include "swapcheck_%s.h"', h)
> +        _hc('#include "swapcheck_xproto.h"')
>  
>      _c('#include "%s.h"', _filename)
>      _c('#include <stdlib.h>')
> -    _c('#include <string.h>')
>      _c('#include <assert.h>')
>      _c('#include <stddef.h>  /* for offsetof() */')
> -    _c('#include "xcb/xcbext.h"')
>      _c('#include <errno.h>')
> -    
> +
>      _c('')
>      _c('#define ALIGNOF(type) offsetof(struct { char dummy; type member; }, member)')
>  
> @@ -196,27 +213,30 @@ def c_open(self):
>          _h('#define XCB_%s_MAJOR_VERSION %s', _ns.ext_name.upper(), _ns.major_version)
>          _h('#define XCB_%s_MINOR_VERSION %s', _ns.ext_name.upper(), _ns.minor_version)
>          _h('') #XXX
> -        _h('extern xcb_extension_t %s;', _ns.c_ext_global_name)
> -        
> +        #_h('extern xcb_extension_t %s;', _ns.c_ext_global_name)
>  
>          _c('')
> -        _c('xcb_extension_t %s = { "%s", 0 };', _ns.c_ext_global_name, _ns.ext_xname)
> -        
> -        _hc("")
> -            
> +        #_c('xcb_extension_t %s = { "%s", 0 };', _ns.c_ext_global_name, _ns.ext_xname)
> +
> +        _hc('')
> +
>  def c_close(self):
>      '''
>      Exported function that handles module close.
>      Writes out all the stored content lines, then closes the files.
>      '''
> -    
> -    for reqnum in requests:
> -        _c('#define %s %s', 'GEN_' + '_'.join(requests[reqnum]).upper(), reqnum)
> -    
> +    _c_setlevel(0)
> +    for reqnum in _requests:
> +        _c('#define %s %s', 'GEN_' + _n(_requests[reqnum]).upper(), reqnum)
> +    _c('')
> +
>      _h_setlevel(2)
>      _c_setlevel(2)
>      _hc('')
>  
> +    for kind in swapCheckDescendants:
> +        generate_dispatch(kind(RequestHandler()), self.namespace.header)
> +
>      _h('')
>      _h('#ifdef __cplusplus')
>      _h('}')
> @@ -228,18 +248,14 @@ def c_close(self):
>      _h('/**')
>      _h(' * @}')
>      _h(' */')
> -    
> +
>      # Ensure the gen subdirectory exists
> -    gendir = 'gen'
>      try:
>          os.mkdir(gendir)
>      except OSError as e:
>          if e.errno != errno.EEXIST:
>              raise
>  
> -    for kind in swapCheckSingletons:
> -        generate_dispatch(kind(RequestHandler()), self.namespace.header)
> -
>      # Write header file
>      hfile = open('%s/%s.h' % (gendir, _filename), 'w')
>      for list in _hlines:
> @@ -254,42 +270,64 @@ def c_close(self):
>          for line in list:
>              cfile.write(line)
>              cfile.write('\n')
> -    
> +
>      cfile.close()
>  
>  def c_simple(self, name):
>      '''
> -    Exported function that handles simple declarations.
> +    Exported function that handles cardinal type declarations.
> +    These are types which are typedef'd to one of the CARDx's, char, float, etc.
>      '''
> -    print(name)
> -    print(self)
> -    
> +    #Needs future implementation
> +
>  
>  def c_enum(self, name):
>      '''
>      Exported function that handles enum declarations.
> +
> +    Private fields:
> +        * fields dictonary contains (enum_number -> enum_name) pair, which
> +            represents enum entry
>      '''
> -    print(self)
> -    print(name)
> -    
> +    _fields = {}
> +
> +    if self.values:
> +        for entry in self.values:
> +            _fields[entry[1]] = entry[0]
> +    else: #if self.bits
> +        for entry in self.bits:
> +            _fields[2**entry[1]] = entry[0] # dunno if it's right
> +
> +    _c('%stypedef enum %s {', _i(), _n(name))
> +    _increase_indent()
> +    for enum_num in _fields:
> +        _c('%s%s = %s,', _i(), (_n(name)+'_'+_fields[enum_num]).upper(), enum_num)
> +
> +    _decrease_indent()
> +    _c('%s} %s_t;', _i(), _n(name))
> +    _c('')
> +
>  def c_struct(self, name):
>      '''
>      Exported function that handles struct declarations.
>      '''
> -    for kind in swapCheckSingletons:
> -        swapcheck = kind(StructHandler())
> -        swapcheck.generate(self, name)
> -        
> +    _h_setlevel(1)
> +    _c_setlevel(1)
> +
> +    for kind in swapCheckDescendants:
> +        _swapcheck = kind(StructHandler())
> +        _swapcheck.generate(self, name)
> +
>  def c_union(self, name):
>      '''
> -    Exported function that handles union declarations.
> +    Exported function that handles union declarations (union is deprecated)
>      '''
> -    print(name)
> -    
> +    #Needs future implementation
> +
>  class SwapCheck:
>      '''
> -    Represent abstract swapper/checker, that generate appropriate c-code        
> -    
> +    Represent abstract swapper/checker, that generates appropriate c-code
> +
>      Created combined because these classes share a lot functions
>      _name is hardcoded literal that represents name of it's class, is used to concatenate it with other stuff to generate functions
>      _docheck is bool that determines accurate position to define afterEnd by calling determine_afterEnd
> @@ -297,126 +335,192 @@ class SwapCheck:
>      _name = None
>      _docheck = False
>      _reusedvars = []
> +    _declaredvars = []
>      _typeHandler = None
> -    
> +    _has_struct = False
> +
>      def __init__(self, typeHandler):
>          self._typeHandler = typeHandler
> -    
> +
>      def access_check( self, size ):
>          if self._docheck:
> -            _c('    if( p + %u > (uint8_t*)afterEnd)\n\t{', size)
> -            _c('        return BadLength;\n\t}')
> -        
> +            _code('%sif( p + %u > (uint8_t*)afterEnd) {', _i(), size)
> +            _increase_indent()
> +            _code('%sreturn BadLength;', _i())
> +            _decrease_indent()
> +            _code('%s}',  _i())
> +
>      def process_fieldvalue(self, fname, ftype):
> -        self._typeHandler.determine_afterEnd(self._docheck, fname)
> -        _c('%r', self._docheck)
> -        if(fname in self._reusedvars):
> -                varname = 'fieldvalue' + '_' + ''.join(fname);
> -                _c('    %s %s = *(%s*)p;', ''.join(ftype.name), varname, ''.join(ftype.name))
> -        
> +        self._docheck = self._typeHandler.determine_afterEnd(self._docheck, fname)
> +        if fname in self._reusedvars:
> +                _varname = 'fieldvalue' + '_' + ''.join(fname)
> +                _datatype = _n(ftype.name)
> +                if(fname not in self._declaredvars):
> +                    _c('%s%s %s;', _i(), _datatype, _varname)
> +                self._declaredvars.append(fname)
> +                _code('%s%s = *(%s*)p;', _i(), _varname, _datatype)
>      def funcName(self, name):
>          return '_'.join(name) + '_' + self.nameToString()
> -        
> +
>      def printHeader(self, type):
> -        #_hc('static int')
>          _hc('int')
>          self._typeHandler.printSwapCheckFuncSignature(self.funcName(type.name))
> -        _c('\t//Field attributes:')
> -        _c('\t//type, field_type, field_name, visible, wire, auto, enum, isfd')
> +        _increase_indent()
> +        _c('%s//type, field_type, field_name, visible, wire, auto, enum, isfd', _i())
>          _c('')
> -        _c('    uint8_t* p = (uint8_t*)data;')
> -        self._typeHandler.initAfterStruct()
> +        _c('%suint8_t* p = (uint8_t*)data;', _i())
> +
> +        if self._has_struct:
> +            self._typeHandler.initAfterStruct()
>          self._typeHandler.init_afterEnd()
> -                
> +
>          _c('')
> -        
> +
>      def printFieldHeader(self, field):
> -        _c('    //%s', field.field_name)
> -        _c('    //%s, %s, %s, %s, %s, %s, %s', field.field_type,
> -                                                field.field_name,
> -                                                field.visible,
> -                                                field.wire,
> -                                                field.auto,
> -                                                field.enum,
> -                                                field.isfd)        
> -    
> +        _code('%s//%s, %s, %s, %s, %s, %s, %s', _i(),
> +                                                field.field_type,
> +                                                field.field_name,
> +                                                field.visible,
> +                                                field.wire,
> +                                                field.auto,
> +                                                field.enum,
> +                                                field.isfd)
> +
>      def process_simple(self, fname, ftype):
>          pass
> -        
> -    def process_fixed_size_list(self, ftype, fname):
> -        varname = 'i'+ '_' + fname
> -        _c('    {')
> -        _c('    int %s;', varname)
> -        _c('    for(%s = 0; %s < %d; %s++)\n\t{', varname, varname, ftype.nmemb, varname)
> -        self.check(ftype.member, None)
> -        _c('    }')
> -        _c('    }')
> -        
> -    def process_varsized_list(self, ftype):
> -        var_iter = 'i'+ '_' + ftype.expr.lenfield_name
> -        var_listlen = 'fieldvalue' + '_' + ftype.expr.lenfield_name
> -        _c('    {')
> -        _c('    unsigned int %s;', var_iter)
> -        _c('    for(%s = 0; %s < %s; %s++)\n\t{', var_iter, var_iter, var_listlen, var_iter)
> +
> +    def process_list(self, ftype, it, llen):
> +        #_c('%s{', _i())
> +        if it not in self._declaredvars:
> +            _c('%sunsigned int %s;', _i(), it)
> +        self._declaredvars.append(it)
> +        #_increase_indent()
> +        _code('%sfor(%s = 0; %s < %s; %s++)', _i(), it, it, llen, it)
> +        _code('%s{', _i())
> +
> +        _increase_indent()
>          self.check(ftype.member, None)
> -        _c('    }')
> -        _c('    }')
> -    
> +        _decrease_indent()
> +
> +        _code('%s}', _i())
> +        #_decrease_indent()
> +        #_code('%s}', _i())
> +
>      def process_struct(self, tname):
>          self._typeHandler.process_struct(self.funcName(tname))
> -        
> +
>      def process_switch(self, ftype):
> -        #self._docheck = True
> -        for field in ftype.fields:
> -            _c('    //%s %s', field.type.name, field.field_name)
> -            #self.printFieldHeader(field.type)
> -            self.check(field.type, field.field_name)
> -        
> +        _casevarn = 'fieldvalue_' + ftype.expr.lenfield_name # case variable name
> +        _code('\n%s//switch begins\n', _i())
> +
> +        for case in ftype.bitcases:
> +            _eq_sign = '==' if case.type.is_case else '&'
> +
> +            if case.type.expr: # if bitcase/case has enumref
> +                _enumn = case.type.expr[0].lenfield_type.name    # enum name
> +                _enument = case.type.expr[0].lenfield_name # enum entry name #TODO WHAT IF NOT 0?
> +            _code('%sif (%s %s %s)', _i(),
> +                                _casevarn, _eq_sign,
> +                                (_n(_enumn)+'_'+_enument).upper())
> +            _code('%s{', _i())
> +            _increase_indent()
> +            for field in case.type.fields:
> +                _code('%s//%s %s', _i(), field.type.name, field.field_name)
> +                self.check(field.type, field.field_name)
> +            _decrease_indent()
> +            _code('%s}', _i())
> +            _code('')
> +
> +        _code('%s//switch ends\n', _i())
> +
>      def check(self, ftype, fname):
> -        _c('%r', self._docheck)
>          _size = ftype.size
>          if ftype.is_simple or ftype.is_expr:
>              self.process_simple(fname, ftype)
> -            _c('    p += %u;', _size)
> +            _code('%sp += %u;', _i(), _size)
>          elif ftype.is_pad and ftype.fixed_size():
> -            _c('    p += %u;', _size)
> +            byts = ftype.nmemb
> +            _code('%sp += %u;', _i(), byts)
> +        elif ftype.is_pad and not ftype.fixed_size():
> +            al = ftype.align
> +            _code('%sp += %u;', _i(), al)
>          elif ftype.is_list and ftype.fixed_size():
> -            self.process_fixed_size_list(ftype, fname)
> +            self.process_list(ftype, 'i_'+fname, ftype.nmemb)
>          elif ftype.is_list and not ftype.fixed_size():
> -            self.process_varsized_list(ftype)
> +            self.process_list(ftype,
> +                                'i_'+ftype.expr.lenfield_name,
> +                                'fieldvalue_' + ftype.expr.lenfield_name)
>          elif ftype.is_switch:
>              self.process_switch(ftype)
>          elif ftype.is_container:
>              self.process_struct(ftype.name)
>          else:
> -            _c(    '    #error swap of nonsimple or nonfixed size fields not implemented')
> -        _c('')                        
> -            
> +            _code(    '%s#error yet not implemented', _i())
> +
>      def generate(self, item, name):
>          self._docheck = self._typeHandler.init_docheck(self._docheck)
>          _afterEnd = None
> -        
> -        self._reusedvars = self.checkReusedVariables(item)
> +
> +        self._reusedvars = self.checkReusedVariables(item, [])
> +        self._declaredvars = []
> +        for field in item.fields:
> +            if self.isAfterStructNedeed(field.type):
> +                break
>          self.printHeader(item)
> +
>          for field in item.fields:
>              self.printFieldHeader(field)
> -            self.check(field.type, field.field_name);
> +            self.check(field.type, field.field_name)
> +            self.printEmpty()
>          self._typeHandler.printFooter()
>          self._typeHandler.printReturn()
> -        
> +        self.directPrintEmpty()
> +        output_code()
> +
>      def nameToString(self):
>          return self._name
> -        
> -    def checkReusedVariables(self, request):
> +
> +    def isAfterStructNedeed(self, ftype):
> +        '''
> +        recurce over every field in request to find out whether it contains structs
> +
> +        to avoid afterEnd is declared but not used warning we iterate over all
> +        fields in the request and if it contains a single struct entry we must
> +        declare it, otherwise we must not.
> +        '''
> +        if ftype.is_list:
> +            self._has_struct = self.isAfterStructNedeed(ftype.member) # check if members of list are structs
> +        elif ftype.is_switch:
> +            for sfield in ftype.fields: # check if any field of switch is a struct
> +                if self.isAfterStructNedeed(sfield.type):
> +                    self._has_struct = True
> +        elif ftype.is_container:
> +            self._has_struct = True
> +        else:
> +            self._has_struct = False
> +        return self._has_struct
> +
> +    def printEmpty(self):
> +        _code('')
> +
> +    def directPrintEmpty(self):
> +        _c('')
> +
> +    def checkReusedVariables(self, _container, other):
> +        appendvars = []
>          listvars = []
> -        othervars = []
> -        for field in request.fields:
> -            if field.type.is_list:
> +        othervars = other
> +        for field in _container.fields:
> +            if field.type.is_list or field.type.is_switch:
>                  listvars.append(field.type.expr.lenfield_name)
> +                if field.type.is_switch:
> +                    appendvars.extend(self.checkReusedVariables(field.type, othervars))
>              else:
>                  othervars.append(field.field_name)
> -        return list(set(listvars) & set(othervars))
> -        
> +        listvars = list(set(listvars) & set(othervars))
> +        listvars.extend(appendvars)
> +        return listvars
> +
>  class SwapParent(SwapCheck):
>      '''
>      Represents abstract class to generate toClient and fromClient swapping functions
> @@ -425,7 +529,7 @@ class SwapParent(SwapCheck):
>          self.before_swap_simplefield(fname, ftype)   #before swap hook
>          self.swap_simplefield(ftype.size)
>          self.after_swap_simplefield(fname, ftype) #after swap hook
> -        
> +
>      def swap_simplefield(self, _size ):
>          if _size == 1:
>              pass
> @@ -439,190 +543,218 @@ class SwapParent(SwapCheck):
>              _c(    '    #error swap of size %d not implemented', _size )
>  
>      def swap_with_swapper( self, swapper, size ):
> -        _c(   '    %s((%s*)p);', swapper, size)
> +        _code('%s%s((%s*)p);', _i(), swapper, size)
>  
>      def before_swap_simplefield( self, fname, ftype ):
>          pass
>  
>      def after_swap_simplefield( self, fname, ftype ):
>          pass
> -    
> +
>  class SwapFromClient(SwapParent):
>      _name = 'SwapFromClient'
> -    
> +
>      def after_swap_simplefield( self, fname, ftype):
>          self.process_fieldvalue( fname, ftype )
> -    
> +
>  class SwapToClient(SwapParent):
>      _name = 'SwapToClient'
> -    
> +
>      def before_swap_simplefield( self, fname, ftype):
>          self.process_fieldvalue( fname,  ftype)
> -        
> +
>  class Check(SwapCheck):
>      _name = 'Check'
> -                
> +
>      def process_simple(self, fname, _size):
> -        self.process_fieldvalue(fname, _size)        
> -    
> +        self.process_fieldvalue(fname, _size)
> +
>  class TypeHandler:
>      def printFooter(self):
>          pass
> -        
> +
>      def determine_afterEnd(self, _docheck, fname):
>          pass
> -        
> +
>      def    printReturn(self):
> -        _c('        return Success;')
> -        _c('    else')
> -        _c('        return BadLength;')
> -        _c('}')
> -        _hc('')    
> -    
> +        _code('        return Success;')
> +        _code('    else')
> +        _code('        return BadLength;')
> +        _code('}')
> +        _decrease_indent()
> +        _h('')
> +        _code('')
> +
>      def printSwapCheckFuncSignature(self, name):
>          pass
> -        
> +
>      def init_afterEnd(self):
>          pass
> -        
> +
>      def init_docheck(self, docheck):
>          pass
> -        
> +
>      def initAfterStruct(self):
>          pass
> -        
> +
>      def process_struct(self, name):
>          pass
> -            
> +
>  class RequestHandler(TypeHandler):
>      def printFooter(self):
> -        _c('    if (p == afterEnd)')
> -        
> +        _code('    if (p == afterEnd)')
> +
>      def determine_afterEnd(self, _docheck, fname):
> +        """
> +        Overloaded function to return the appropriate value of _docheck,
> +
> +        unlike the StructHandler does
> +        """
>          if fname == 'length':
> -            _c('    afterEnd = ((uint8_t*)data) + 4 *  ( *(uint16_t*)p );')
> -            _docheck = True
> -            
> +            _code('    afterEnd = ((uint8_t*)data) + 4 *  ( *(uint16_t*)p );')
> +            return True
> +        else:
> +            return _docheck
>      def printSwapCheckFuncSignature(self, name):
>          _h('%s(void *data);', name)
>          _c('%s(void *data)\n{', name)
> -        
> +
>      def init_afterEnd(self):
>          _c('    uint8_t* afterEnd = NULL;')
> -        
> +
>      def init_docheck(self, docheck):
>          return False
> -    
> +
>      def initAfterStruct(self):
>          _c('    uint8_t* afterStruct = NULL;')
> -    
> +
>      def process_struct(self, name):
> -        _c('        if(%s(p, afterEnd, &afterStruct) != Success)\n\t\t\treturn BadLength;', name)
> -        _c('        p = afterStruct;')
> -            
> +        _code('%sif(%s(p, afterEnd, &afterStruct) != Success)\n\t\t\treturn BadLength;', _i(), name)
> +        _code('%sp = afterStruct;', _i())
> +
>  class StructHandler(TypeHandler):
>      def printFooter(self):
> -        _c('    *afterStruct = p;')
> -        _c('    if (p <= (uint8_t*)afterEnd)')
> -        
> +        _code('    *afterStruct = p;')
> +        _code('    if (p <= (uint8_t*)afterEnd)')
> +
>      def printSwapCheckFuncSignature(self, name):
>          _h('%s(void *data, void *afterEnd, uint8_t **afterStruct);', name)
>          _c('%s(void *data, void *afterEnd, uint8_t **afterStruct)\n{', name)
> -        
> +
>      def init_docheck(self, docheck):
>          return True
> -        
> +
>      def process_struct(self, name):
> -        _c('        if(%s(p, afterEnd, afterStruct) != Success)\n\t\t\treturn BadLength;', name)
> -        _c('        p = (uint8_t*)(*afterStruct);')
> -        
> +        _code('%sif(%s(p, afterEnd, afterStruct) != Success)\n\t\t\treturn BadLength;', _i(), name)
> +        _code('%sp = (uint8_t*)(*afterStruct);', _i())
> +
>      def determine_afterEnd(self, _docheck, fname):
> -        pass
> -                
> +        """
> +        Overloaded function to return the same value as _docheck had,
> +
> +        unlike the RequestHandler's one
> +        """
> +        return _docheck
> +
>  def generate_dispatch(kind, name):
> -    #_c('static int')
> -    _c('int')
> -    _c('xcb_%s_dispatch(void *req)\n{', kind._name)
> -    _c('    long switch_item;')
> -    _c('    int return_val = 0;')
> -    _c('    xReq* request = (xReq*)req;')
> -    _c('    const char *name = "%s";\n', name)
> -    _c('    if (strcmp(name, "xproto") == 0)')
> -    _c('        switch_item = request->reqType;')
> -    _c('    else switch_item = request->data;\n')
> -    _c('    switch(switch_item)\n\t{')
> -    
> -    global requests
> -    for reqnum in requests:
> -        _c('\t\tcase %s:\n\t\t\treturn_val = %s((void*)request);\n\t\t\tbreak;', 'GEN_'+'_'.join(requests[reqnum]).upper(),  kind.funcName(requests[reqnum]) )
> -    _c('    }\n')
> -    _c('    return return_val;')
> -    _c('}\n')
> +    """
> +    Function to generate dispatch function
> +
> +    kind is an object of class SwapCheck
> +    name is the name of extension
> +    """
> +    #print function header-signature
> +    _hc('%sint', _i())
> +    _h('%sxcb_%s_dispatch(void *req);', _i(), name+'_'+kind._name)
> +    _c('%sxcb_%s_dispatch(void *req)\n{', _i(), name+'_'+kind._name)
> +    _increase_indent()
> +
> +    # init variables according to c90 std
> +    _c('%slong switch_item;', _i())
> +    _c('%sint return_val = 0;', _i())
> +    _c('%sxReq* request = (xReq*)req;', _i())
> +
> +    # define varible to switch, depends on extension
> +    _switch_item = 'request->'
> +    _switch_item = _switch_item + 'reqType' if name == 'xproto' else _switch_item + 'data'
> +    _c('%sswitch_item = %s;', _i(), _switch_item)
> +
> +    # print switch header
> +    _c('%sswitch(switch_item)', _i())
> +    _c('%s{', _i())
> +    _increase_indent()
> +
> +    # for every opcode in generated (opcode -> name) dict "_requests"
> +    # print case
> +    for reqnum in _requests:
> +        _c('%scase %s:', _i(), 'GEN_'+_n(_requests[reqnum]).upper())
> +        _increase_indent()
> +        _c('%sreturn_val = %s((void*)request);', _i(),
> +                                                kind.funcName(_requests[reqnum]))
> +        _c('%sbreak;', _i())
> +        _decrease_indent()
> +    _c('%sdefault:', _i())
> +    _increase_indent()
> +    _c('%sreturn BadRequest;', _i())
> +    _decrease_indent()
> +
> +    _c('%s}', _i())
> +    _decrease_indent()
> +    _c('%sreturn return_val;', _i())
> +    _decrease_indent()
> +    _c('%s}', _i())
>  
>  def c_request(self, name):
>      '''
>      Exported function that handles request declarations.
> -    '''
> -    global requests
> -    requests[self.opcode] = name
> -    
> -    for kind in swapCheckSingletons:
> +    '''
> +    _h_setlevel(1)
> +    _c_setlevel(1)
> +    global _requests
> +    _requests[self.opcode] = name
> +    for kind in swapCheckDescendants:
>          swapcheck = kind(RequestHandler())
>          swapcheck.generate(self, name)
> -        
> -swapCheckSingletons = {SwapFromClient, SwapToClient, Check}
> -    
> +
> +swapCheckDescendants = {SwapFromClient, SwapToClient, Check}
> +
>  def c_event(self, name):
>      '''
>      Exported function that handles event declarations.
>      '''
> -    print(name)
> -    
> +    #Needs future implementation
> +
>  def c_error(self, name):
>      '''
>      Exported function that handles error declarations.
>      '''
> -    print(self)
> -    print(name)
> -    
> -# Must create an "output" dictionary before any xcbgen imports.
> -output = {'open'    : c_open,
> -          'close'   : c_close,
> -          'simple'  : c_simple,
> -          'enum'    : c_enum,
> -          'struct'  : c_struct,
> -          'union'   : c_union,
> -          'request' : c_request,
> -          'event'   : c_event,
> -          'error'   : c_error,
> -          }
> -
> -# Check for the argument that specifies path to the xcbgen python package.
> -try:
> -    opts, args = getopt.getopt(sys.argv[1:], 'p:')
> -except getopt.GetoptError as err:
> -    print(err)
> -    print('Usage: basic_framework.py [-p path] file.xml')
> -    sys.exit(1)
> +    #Needs future implementation
>  
> -for (opt, arg) in opts:
> -    if opt == '-p':
> -        sys.path.insert(1, arg)
> +# Must create an "output" dictionary before any xcbgen imports.
> +output = {'open'    : c_open,
> +          'close'   : c_close,
> +          'simple'  : c_simple,
> +          'enum'    : c_enum,
> +          'struct'  : c_struct,
> +          'union'   : c_union,
> +          'request' : c_request,
> +          'event'   : c_event,
> +          'error'   : c_error,
> +          }
>  
>  # Import the module class
>  try:
> -    from xcbgen.state import Module
> -    from xcbgen.xtypes import *
> +    from xcbgen.state import Module
> +    from xcbgen.xtypes import *
>  except ImportError:
> -    print('''
> +    print('''
>  Failed to load the xcbgen Python package!
>  Make sure that xcb/proto installed it on your Python path.
>  If not, you will need to create a .pth file or define $PYTHONPATH
>  to extend the path.
>  Refer to the README file in xcb/proto for more info.
>  ''')
> -    raise
> -
> +    raise
>  module = Module(args[0], output)
>  module.register()
>  module.resolve()
> -- 
> 1.9.1



On 03/10/15 19:51, Asal Mirzaieva wrote:
>
>
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel



More information about the xorg-devel mailing list