Good morning all and Peter, I replied to your comments below.
On 17.03.15 09:25, Peter Hutterer wrote:
On Wed, Mar 11, 2015 at 08:46:41PM +0200,[email protected] wrote:--- .gitignore | 3 + Makefile.am | 4 +- Xext/Makefile.am | 2 +- Xext/shape.c | 167 ++--------- configure.ac | 41 ++- proto/.gitignore | 5 + proto/Makefile.am | 21 ++ proto/gen_swap_check.py | 761 ++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 860 insertions(+), 144 deletions(-) create mode 100644 proto/.gitignore create mode 100644 proto/Makefile.am create mode 100644 proto/gen_swap_check.py diff --git a/.gitignore b/.gitignore index dc56b46..b45e8cb 100644 --- a/.gitignore +++ b/.gitignore @@ -80,3 +80,6 @@ core doltcompile doltlibtool xserver.ent + +# vim swap files +*.swpthis can/should be a separate patch
OK.
You mean, I don't need AC_ARG_ENABLE? I just don't get how to intergrate the newly written code to already existing one another way.diff --git a/Makefile.am b/Makefile.am index f0fa2d8..aac0a69 100644 --- a/Makefile.am +++ b/Makefile.am @@ -42,6 +42,7 @@ SUBDIRS = \ dix \ fb \ mi \ + proto \ Xext \ miext \ os \ @@ -116,7 +117,8 @@ DIST_SUBDIRS = \ dri3 \ present \ hw \ - test + test \ + proto# gross hackrelink: all 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 XORGsdk_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..f1e59aa 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,36 @@ 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,141 +1118,6 @@ 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; - } -} - void ShapeExtensionInit(void) { diff --git a/configure.ac b/configure.ac index 96524c5..5b89705 100644 --- a/configure.ac +++ b/configure.ac @@ -33,6 +33,7 @@ AC_CONFIG_SRCDIR([Makefile.am]) AC_CONFIG_MACRO_DIR([m4]) AM_INIT_AUTOMAKE([foreign dist-bzip2]) AC_USE_SYSTEM_EXTENSIONS +AM_PATH_PYTHON([2.7])# Require xorg-macros minimum of 1.14 for XORG_COMPILER_BRAND in XORG_DEFAULT_OPTIONSm4_ifndef([XORG_MACROS_VERSION], @@ -576,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])I'm gonna go out on a limb here and say that the help string isn't quite what you were looking for :)fwiw, "proto" is a rather generic term, I'd use xcb-proto-parsing orsomething. But really, I think this should be unconditional anyway so you don't need this flag at all.
I thought, that it can be a reserved word or a variable somewhere else, so I didn't want to risk. I guess, I should have written something like xcb_proto_parsing_prefix = swapcheck_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]) @@ -1036,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 @@ -1776,7 +1785,26 @@ 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 +PKG_CHECK_MODULES([XCBPROTO], [xcb-proto]) +XCBPROTO_XCBINCLUDEDIR=`$PKG_CONFIG --variable=xcbincludedir xcb-proto` +AC_SUBST(XCBPROTO_XCBINCLUDEDIR) + +# 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 +PROTO_CFLAGS="$XSERVER_CFLAGS $XCBPROTO_CFLAGS" +AC_SUBST([PROTO_CFLAGS])dnl ---------------------------------------------------------------------------dnl DDX section. @@ -1789,7 +1817,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]) @@ -1810,7 +1838,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]) @@ -1835,7 +1863,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 @@ -2391,11 +2419,11 @@ if test "$KDRIVE" = yes; then 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*) @@ -2546,6 +2574,7 @@ xfixes/Makefile exa/Makefile dri3/Makefile present/Makefile +proto/Makefile hw/Makefile hw/xfree86/Makefile hw/xfree86/Xorg.sh diff --git a/proto/.gitignore b/proto/.gitignore new file mode 100644 index 0000000..cc4611b --- /dev/null +++ b/proto/.gitignore @@ -0,0 +1,5 @@ +# Python "compiled" files +*.pyc + +# Autogenerated by gen_swap_check.py in proto files +gen/* diff --git a/proto/Makefile.am b/proto/Makefile.am new file mode 100644 index 0000000..34af921 --- /dev/null +++ b/proto/Makefile.am @@ -0,0 +1,21 @@ +noinst_LTLIBRARIES = libproto.la + +AM_CFLAGS = $(PROTO_CFLAGS) + +prefx=swapcheck_is prefix a reserved word? or do we just disagree about the spelling? :)
+PROTO_GENERATEDDIR=generated + +extension_sources = \ + ${PROTO_GENERATEDDIR}/${prefx}xproto.c \ + ${PROTO_GENERATEDDIR}/${prefx}shape.c + +libproto_la_SOURCES = $(extension_sources) + +sources = $(subst ${PROTO_GENERATEDDIR}/${prefx}, ,$(@)) + +$(extension_sources): $(XCBPROTO_XCBINCLUDEDIR)/$(sources:.c=.xml) $(srcdir)/gen_swap_check.py +you can drop this empty line here+ $(AM_V_GEN)$(PYTHON) $(srcdir)/gen_swap_check.py \ + -p $(XCBPROTO_XCBPYTHONDIR) \ + -d ${PROTO_GENERATEDDIR} \ + $(XCBPROTO_XCBINCLUDEDIR)/$(sources:.c=.xml) diff --git a/proto/gen_swap_check.py b/proto/gen_swap_check.py new file mode 100644 index 0000000..f0ca86e --- /dev/null +++ b/proto/gen_swap_check.py @@ -0,0 +1,761 @@ +#!/usr/bin/env python +import getopt +import os +import sys +import errno +import re + +def _i(): + ''' + Indent function + + Returns joined _indent to be concatenated with code string in _c() + ''' + return ''.join(_indent) + +# 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) + +def _c(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 _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 = {} +_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 + +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 + +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) + +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() + +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() + +# 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. + ''' + 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) + + _hc('/*') + _hc(' * This file generated automatically from %s by gen_swap_check.py.', _ns.file) + _hc(' * Edit at your peril.') + _hc(' */') + _hc('') + + _h('/**') + _h(' * @defgroup XCB_%s_API XCB %s API', _ns.ext_name, _ns.ext_name) + _h(' * @brief %s XCB Protocol Implementation.', _ns.ext_name) + _h(' * @{') + _h(' **/') + _h('') + _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 "xorg/misc.h"') + _h('#include "X11/X.h"') + _h('#include "X11/Xproto.h"') + else: + _hc('#include "swapcheck_xproto.h"') + + _c('#include "%s.h"', _filename) + _c('#include <stdlib.h>') + _c('#include <assert.h>') + _c('#include <stddef.h> /* for offsetof() */') + _c('#include <errno.h>') + + _c('') + _c('#define ALIGNOF(type) offsetof(struct { char dummy; type member; }, member)') + + _h('') + _h('#ifdef __cplusplus') + _h('extern "C" {') + _h('#endif') + + if _ns.is_ext: + _h('') + _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) + + _c('') + #_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. + ''' + _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('}') + _h('#endif') + + _h('') + _h('#endif') + _h('') + _h('/**') + _h(' * @}') + _h(' */') + + # Ensure the gen subdirectory exists + try: + os.mkdir(gendir) + except OSError as e: + if e.errno != errno.EEXIST: + raise + + # Write header file + hfile = open('%s/%s.h' % (gendir, _filename), 'w') + for list in _hlines: + for line in list: + hfile.write(line) + hfile.write('\n') + hfile.close() + + # Write source file + cfile = open('%s/%s.c' % (gendir, _filename), 'w') + for list in _clines: + for line in list: + cfile.write(line) + cfile.write('\n') + + cfile.close() + +def c_simple(self, name): + ''' + Exported function that handles cardinal type declarations. + These are types which are typedef'd to one of the CARDx's, char, float, etc. + ''' + #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 + ''' + _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. + ''' + _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 (union is deprecated) + ''' + #Needs future implementation + +class SwapCheck: + ''' + 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_afterEndthis should be linewrapped. I'm also struggling - where is the copied code and where is the new code? again, this should be marked with comments to help focusing on the actual review
The copied code begins starts with " # Helping functions and definitions copy-pasted from c_client.py " comment, and ends with " # Copy-pasted from c_client.py code ends here "Could you please clarify what is "linewrapped"? Wrapped with comment? Or indents? Or else?
+ ''' + _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: + _code('%sif( p + %u > (uint8_t*)afterEnd) {', _i(), size) + _increase_indent() + _code('%sreturn BadLength;', _i()) + _decrease_indent() + _code('%s}', _i())general nitpick: it's usually better to invert a condition and return/break/continue early than have the whole block indented. so in this case you'd do a if not self._docheck: return .... other code
Fixed.
I assumed tab = 4 spaces, as it is defined in http://www.x.org/wiki/CodingStyle/. As python strictly requires tabs and not space-indents, I thought it would be the nicer way to organize code. But I will switch to tab = 8 spaces and fix all the tabs issues.+ + def process_fieldvalue(self, fname, ftype): + 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)different indentation to the rest of the code, and the above comment applies here too+ def funcName(self, name): + return '_'.join(name) + '_' + self.nameToString() + + def printHeader(self, type): + _hc('int') + self._typeHandler.printSwapCheckFuncSignature(self.funcName(type.name)) + _increase_indent() + _c('%s//type, field_type, field_name, visible, wire, auto, enum, isfd', _i()) + _c('') + _c('%suint8_t* p = (uint8_t*)data;', _i()) + + if self._has_struct: + self._typeHandler.initAfterStruct() + self._typeHandler.init_afterEnd() + + _c('') + + def printFieldHeader(self, field): + _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)whoah, that is a lot of tabs...
+ + def process_simple(self, fname, ftype): + pass + + 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) + _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): + _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): + _size = ftype.size + if ftype.is_simple or ftype.is_expr: + self.process_simple(fname, ftype) + _code('%sp += %u;', _i(), _size) + elif ftype.is_pad and ftype.fixed_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_list(ftype, 'i_'+fname, ftype.nmemb) + elif ftype.is_list and not ftype.fixed_size(): + self.process_list(ftype, + 'i_'+ftype.expr.lenfield_name, + 'fieldvalue_' + ftype.expr.lenfield_name)indentation+ elif ftype.is_switch: + self.process_switch(ftype) + elif ftype.is_container: + self.process_struct(ftype.name) + else: + _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._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.printEmpty() + self._typeHandler.printFooter() + self._typeHandler.printReturn() + self.directPrintEmpty() + output_code() + + def nameToString(self): + return self._name + + 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 = 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) + listvars = list(set(listvars) & set(othervars)) + listvars.extend(appendvars) + return listvars + +class SwapParent(SwapCheck): + ''' + Represents abstract class to generate toClient and fromClient swapping functions + ''' + def process_simple(self, fname, ftype): + 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 + elif _size == 2: + self.access_check(_size ) + self.swap_with_swapper('swaps', 'uint16_t') + elif _size ==4 : + self.access_check( _size ) + self.swap_with_swapper('swapl', 'uint32_t') + else: + _c( ' #error swap of size %d not implemented', _size ) + + def swap_with_swapper( self, 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) + +class TypeHandler: + def printFooter(self): + pass + + def determine_afterEnd(self, _docheck, fname): + pass + + def printReturn(self):whitespace issue here
Fixed.
+ _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): + _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': + _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;')shouldn't you be using _i() here instead of the tab? in the generated code Chris sent late Feb this line is always over-indented
Fixed
+ + def init_docheck(self, docheck): + return False + + def initAfterStruct(self): + _c(' uint8_t* afterStruct = NULL;')same here+ + def process_struct(self, name): + _code('%sif(%s(p, afterEnd, &afterStruct) != Success)\n\t\t\treturn BadLength;', _i(), name)you need to use _i() consistently here, and best to have multiple print lines. otherwise the indentation is messed up
Fixed
+ _code('%sp = afterStruct;', _i()) + +class StructHandler(TypeHandler): + def printFooter(self): + _code(' *afterStruct = p;') + _code(' if (p <= (uint8_t*)afterEnd)')same here
Fixed
I haven't run the X test suit, but so far even running the gnome-terminal fails, though xeyes and xterm work OK, both with and without byte-swapping. I am working on gnome-terminal BadLength bug these days. I will post a patch after clarifying the questions above, so it fully covers the topics of this letterafaict the output is correct, so this looks good. going through the final patch with a fine-tooth-comb will be interesting. Have you run the X test suite against this by any chance? That's probably the most reliable check to see if something is buggy.
Cheers, Peter+ + 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): + _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): + """ + Overloaded function to return the same value as _docheck had, + + unlike the RequestHandler's one + """ + return _docheck + +def generate_dispatch(kind, name): + """ + 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. + ''' + _h_setlevel(1) + _c_setlevel(1) + global _requests + _requests[self.opcode] = name + for kind in swapCheckDescendants: + swapcheck = kind(RequestHandler()) + swapcheck.generate(self, name) + +swapCheckDescendants = {SwapFromClient, SwapToClient, Check} + +def c_event(self, name): + ''' + Exported function that handles event declarations. + ''' + #Needs future implementation + +def c_error(self, name): + ''' + Exported function that handles error declarations. + ''' + #Needs future implementation + +# 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 * +except ImportError: + 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 +module = Module(args[0], output) +module.register() +module.resolve() +module.generate() -- 1.9.1
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
