[PATCH v2 2/5] dix: Enable client ID tracking in server.

[Vignatti Tiago (Nokia-MS/Helsinki)]

On Fri, Sep 10, 2010 at 06:58:39PM +0200, ext Rami Ylimäki wrote:
> Let X server to keep track of client PIDs and process names. Also make
> the client tracking interface available for external modules. Linking
> order of Xnest libraries needs to be fixed, because libmain depends on
> libdix and not vice versa.
> 
> Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
> ---
>  configure.ac                 |   12 +++++++++++-
>  dix/Makefile.am              |    5 ++++-
>  dix/main.c                   |    7 +++++++
>  hw/xfree86/loader/sdksyms.sh |    3 +++
>  include/Makefile.am          |    1 +
>  5 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 1a1f2d3..586df80 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -999,6 +999,16 @@ if test "x$RES" = xyes; then
>  	REQUIRED_MODULES="$REQUIRED_MODULES $RESOURCEPROTO"
>  fi
>  
> +AC_MSG_CHECKING([whether to track client ids])
> +if test "x$RES" = xyes -o "x$XSELINUX" = xyes; then
> +	CLIENTIDS=yes
> +	AC_DEFINE(CLIENTIDS, 1, [Support client id tracking])
> +else
> +	CLIENTIDS=no
> +fi
> +AC_MSG_RESULT([$CLIENTIDS])
> +AM_CONDITIONAL(CLIENTIDS, [test "x$CLIENTIDS" = xyes])

client tracking is a framework for analysis and testing. Not everyone will
want this to be included in products, so I'd make an option to choose at
compile whether is desired or not to have it.


>  if test "x$GLX" = xyes; then
>  	PKG_CHECK_MODULES([XLIB], [x11])
>  	PKG_CHECK_MODULES([GL], $GLPROTO $LIBGL)
> @@ -1527,7 +1537,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 $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $DIX_LIB $MAIN_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 $MIEXT_DAMAGE_LIB $MIEXT_SHADOW_LIB $XI_LIB $XKB_LIB $XKB_STUB_LIB $COMPOSITE_LIB $MAIN_LIB $DIX_LIB $OS_LIB"
>  	XNEST_SYS_LIBS="$XNESTMODULES_LIBS $GLX_SYS_LIBS"
>  	AC_SUBST([XNEST_LIBS])
>  	AC_SUBST([XNEST_SYS_LIBS])
> diff --git a/dix/Makefile.am b/dix/Makefile.am
> index bc87da5..a74e219 100644
> --- a/dix/Makefile.am
> +++ b/dix/Makefile.am
> @@ -7,7 +7,6 @@ libmain_la_SOURCES =    \
>  
>  libdix_la_SOURCES = 	\
>  	atom.c		\
> -	client.c	\
>  	colormap.c	\
>  	cursor.c	\
>  	deprecated.c	\
> @@ -42,6 +41,10 @@ libdix_la_SOURCES = 	\
>  	tables.c	\
>  	window.c
>  
> +if CLIENTIDS
> +libdix_la_SOURCES += client.c
> +endif
> +
>  EXTRA_DIST = buildatoms BuiltInAtoms Xserver.d Xserver-dtrace.h.in
>  
>  # Install list of protocol names
> diff --git a/dix/main.c b/dix/main.c
> index 5c46dc1..3362058 100644
> --- a/dix/main.c
> +++ b/dix/main.c
> @@ -104,6 +104,7 @@ Equipment Corporation.
>  #include "extnsionst.h"
>  #include "privates.h"
>  #include "registry.h"
> +#include "client.h"

I think you forgot to wrap by CLIENTIDS.


>  #ifdef PANORAMIX
>  #include "panoramiXsrv.h"
>  #else
> @@ -260,6 +261,9 @@ int main(int argc, char *argv[], char *envp[])
>          InitCoreDevices();
>  	InitInput(argc, argv);
>  	InitAndStartDevices();
> +#ifdef CLIENTIDS
> +	InitClientIds(serverClient);
> +#endif /* CLIENTIDS */

this kind of wrapping pollutes quite a lot the bulk the file, harming the
readability for instance. Probably a better way is to hide those using dummy
functions like 

void InitClientIds(void) {}

and define them somewhere else when CLIENTIDS is not defined. Take a look on
the bottom of xf86VGAarbiter.c as example.

  
>  	dixSaveScreens(serverClient, SCREEN_SAVER_FORCER, ScreenSaverReset);
>  
> @@ -325,6 +329,9 @@ int main(int argc, char *argv[], char *envp[])
>  	    screenInfo.numScreens = i;
>  	}
>  
> +#ifdef CLIENTIDS
> +	CloseClientIds(serverClient);
> +#endif /* CLIENTIDS */
>  	dixFreePrivates(serverClient->devPrivates, PRIVATE_CLIENT);
>  	serverClient->devPrivates = NULL;
>  
> diff --git a/hw/xfree86/loader/sdksyms.sh b/hw/xfree86/loader/sdksyms.sh
> index 13c5ae5..8da4341 100755
> --- a/hw/xfree86/loader/sdksyms.sh
> +++ b/hw/xfree86/loader/sdksyms.sh
> @@ -264,6 +264,9 @@ cat > sdksyms.c << EOF
>  #include "colormap.h"
>  #include "colormapst.h"
>  #include "hotplug.h"
> +#if CLIENTIDS
> +#include "client.h"
> +#endif
>  #include "cursor.h"
>  #include "cursorstr.h"
>  #include "dix.h"
> diff --git a/include/Makefile.am b/include/Makefile.am
> index e76de05..06cf46f 100644
> --- a/include/Makefile.am
> +++ b/include/Makefile.am
> @@ -4,6 +4,7 @@ sdk_HEADERS =		\
>  	bstore.h	\
>  	bstorestr.h	\
>  	callback.h	\
> +	client.h	\

I don't think client.h should be going to sdk. IIRC the first plan was to
access client tracking by protocol calls (XRes), right?

>  	closestr.h	\
>  	closure.h	\
>  	colormap.h	\
> -- 
> 1.6.3.3
> 
             Tiago