[PATCH xinit] xcb port of xinit. Unsure of correctness in removing error handling in shutdown

Mikhail Gusarov dottedmag at dottedmag.net
Fri Mar 11 00:28:18 PST 2011


Twas brillig at 22:59:49 10.03.2011 UTC+00 when gunkmute at gmail.com did gyre and gimble:

FYI: I've got similar patch (not submitted upstream as it broke
Xfree86_VT functionality here:
http://git.iplinux.org/xinit.git/commit/?id=9bae70664b1dfb2731fc6ceaab6dbf1b7acaf783

 DR> Signed-off-by: Demur Rumed <gunkmute at gmail.com>
 DR> ---
 DR>  configure.ac |    6 ++--
 DR>  xinit.c      |   95 +++++++++++++++++++++++++++-------------------------------
 DR>  2 files changed, 47 insertions(+), 54 deletions(-)

 DR> diff --git a/configure.ac b/configure.ac
 DR> index 67214cb..2a9d5da 100644
 DR> --- a/configure.ac
 DR> +++ b/configure.ac
 DR> @@ -1,5 +1,5 @@
 DR>  dnl  Copyright 2005 Red Hat, Inc.
 DR> -dnl 
 DR> +dnl

Please don't mix whitespace and semantic changes in single patch.

 DR>  dnl  Permission to use, copy, modify, distribute, and sell this software and its
 DR>  dnl  documentation for any purpose is hereby granted without fee, provided that
 DR>  dnl  the above copyright notice appear in all copies and that both that
 DR> @@ -9,7 +9,7 @@ dnl  advertising or publicity pertaining to distribution of the software without
 DR>  dnl  specific, written prior permission.  Red Hat makes no
 DR>  dnl  representations about the suitability of this software for any purpose.  It
 DR>  dnl  is provided "as is" without express or implied warranty.
 DR> -dnl 
 DR> +dnl
 DR>  dnl  RED HAT DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
 DR>  dnl  INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
 DR>  dnl  EVENT SHALL RED HAT BE LIABLE FOR ANY SPECIAL, INDIRECT OR
 DR> @@ -134,7 +134,7 @@ AM_CONDITIONAL(LAUNCHD, [test "x$LAUNCHD" = "xyes"])
 DR>  AM_CONDITIONAL(TIGER_LAUNCHD, [test "x$TIGER_LAUNCHD" = "xyes"])
 DR>  
 DR>  # Checks for pkg-config packages
 DR> -PKG_CHECK_MODULES(XINIT, x11)
 DR> +PKG_CHECK_MODULES(XINIT, xcb)
 DR>  
 DR>  case $host_os in
 DR>      *bsd*)
 DR> diff --git a/xinit.c b/xinit.c
 DR> index 42ff008..8e50d78 100644
 DR> --- a/xinit.c
 DR> +++ b/xinit.c
 DR> @@ -28,9 +28,8 @@ in this Software without prior written authorization from The Open Group.
 DR>  # include "config.h"
 DR>  #endif
 DR>  
 DR> -#include <X11/Xlib.h>
 DR> +#include <xcb/xcb.h>
 DR>  #include <X11/Xos.h>
 DR> -#include <X11/Xatom.h>
 DR>  #include <stdio.h>
 DR>  #include <ctype.h>
 DR>  #include <stdint.h>
 DR> @@ -38,7 +37,6 @@ in this Software without prior written authorization from The Open Group.
 DR>  #include <signal.h>
 DR>  #include <sys/wait.h>
 DR>  #include <errno.h>
 DR> -#include <setjmp.h>
 DR>  #include <stdarg.h>
 DR>  
 DR>  #ifdef __APPLE__
 DR> @@ -101,18 +99,17 @@ static char **server = serverargv + 2;        /* make sure room for sh .xserverr
 DR>  static char **client = clientargv + 2;        /* make sure room for sh .xinitrc args */
 DR>  static char *displayNum = NULL;
 DR>  static char *program = NULL;
 DR> -static Display *xd = NULL;            /* server connection */
 DR> +static xcb_connection_t *xd = NULL;            /* server connection */
 DR>  int status;
 DR>  int serverpid = -1;
 DR>  int clientpid = -1;
 DR>  volatile int gotSignal = 0;
 DR>  
 DR>  static void Execute(char **vec);
 DR> -static Bool waitforserver(void);
 DR> -static Bool processTimeout(int timeout, char *string);
 DR> +static int waitforserver(void);
 DR> +static int processTimeout(int timeout, char *string);
 DR>  static int startServer(char *server[]);
 DR>  static int startClient(char *client[]);
 DR> -static int ignorexio(Display *dpy);
 DR>  static void shutdown(void);
 DR>  static void set_environment(void);
 DR>  
 DR> @@ -134,13 +131,13 @@ sigIgnore(int sig)
 DR>  }
 DR>  
 DR>  static void
 DR> -Execute(char **vec)                /* has room from up above */
 DR> +Execute(char **vec)        /* has room from up above */

Ditto.

 DR>  {
 DR>      execvp(vec[0], vec);
 DR>      if (access(vec[0], R_OK) == 0) {
 DR> -        vec--;                                /* back it up to stuff shell in */
 DR> -        vec[0] = SHELL;
 DR> -        execvp(vec[0], vec);
 DR> +    vec--;                /* back it up to stuff shell in */
 DR> +    vec[0] = SHELL;
 DR> +    execvp(vec[0], vec);

Ditto.

 DR>      }
 DR>      return;
 DR>  }
 DR> @@ -215,12 +212,12 @@ main(int argc, char *argv[])
 DR>       */
 DR>      if (!client_given) {
 DR>          char *cp;
 DR> -        Bool required = False;
 DR> +        int required = FALSE;

Where does this FALSE come from? xcb? Why not bool / true / false with
autoconf check?

 DR>  
 DR>          xinitrcbuf[0] = '\0';
 DR>          if ((cp = getenv("XINITRC")) != NULL) {
 DR>              snprintf(xinitrcbuf, sizeof(xinitrcbuf), "%s", cp);
 DR> -            required = True;
 DR> +            required = TRUE;
 DR>          } else if ((cp = getenv("HOME")) != NULL) {
 DR>              snprintf(xinitrcbuf, sizeof(xinitrcbuf),
 DR>                       "%s/%s", cp, XINITRC);
 DR> @@ -241,12 +238,12 @@ main(int argc, char *argv[])
 DR>       */
 DR>      if (!server_given) {
 DR>          char *cp;
 DR> -        Bool required = False;
 DR> +        int required = FALSE;
 DR>  
 DR>          xserverrcbuf[0] = '\0';
 DR>          if ((cp = getenv("XSERVERRC")) != NULL) {
 DR>              snprintf(xserverrcbuf, sizeof(xserverrcbuf), "%s", cp);
 DR> -            required = True;
 DR> +            required = TRUE;
 DR>          } else if ((cp = getenv("HOME")) != NULL) {
 DR>              snprintf(xserverrcbuf, sizeof(xserverrcbuf),
 DR>                       "%s/%s", cp, XSERVERRC);
 DR> @@ -331,7 +328,7 @@ main(int argc, char *argv[])
 DR>  /*
 DR>   *    waitforserver - wait for X server to start up
 DR>   */
 DR> -static Bool
 DR> +static int
 DR>  waitforserver(void)
 DR>  {
 DR>      int    ncycles     = 120;        /* # of cycles to wait */
 DR> @@ -348,7 +345,7 @@ waitforserver(void)
 DR>  #endif
 DR>  
 DR>      for (cycles = 0; cycles < ncycles; cycles++) {
 DR> -        if ((xd = XOpenDisplay(displayNum))) {
 DR> +        if ((xd = xcb_connect(displayNum, 0))) {

You need here && !xcb_connection_has_error(xd) as well.

 DR>              return(TRUE);
 DR>          }
 DR>          else {
 DR> @@ -365,7 +362,7 @@ waitforserver(void)
 DR>  /*
 DR>   * return TRUE if we timeout waiting for pid to exit, FALSE otherwise.
 DR>   */
 DR> -static Bool
 DR> +static int
 DR>  processTimeout(int timeout, char *string)
 DR>  {
 DR>      int    i = 0, pidfound = -1;
 DR> @@ -479,60 +476,68 @@ static void
 DR>  setWindowPath(void)
 DR>  {
 DR>      /* setting WINDOWPATH for clients */
 DR> -    Atom prop;
 DR> -    Atom actualtype;
 DR> +    xcb_get_property_reply_t *proprep;
 DR> +    xcb_intern_atom_reply_t *atomrep;
 DR> +    xcb_atom_t prop;
 DR> +    xcb_atom_t actualtype;
 DR>      int actualformat;
 DR>      unsigned long nitems;
 DR>      unsigned long bytes_after;
 DR> -    unsigned char *buf;
 DR> +    void *buf;
 DR>      const char *windowpath;
 DR>      char *newwindowpath;
 DR>      unsigned long num;
 DR>      char nums[10];
 DR>      int numn;
 DR>      size_t len;
 DR> -    prop = XInternAtom(xd, "XFree86_VT", False);
 DR> -    if (prop == None) {
 DR> +    atomrep = xcb_intern_atom_reply(xd, xcb_intern_atom_unchecked(xd, FALSE, 10, "XFree86_VT"), NULL);
 DR> +    if (!atomrep) {
 DR>          Errorx("Unable to intern XFree86_VT atom");
 DR>          return;
 DR>      }
 DR> -    if (XGetWindowProperty(xd, DefaultRootWindow(xd), prop, 0, 1,
 DR> -        False, AnyPropertyType, &actualtype, &actualformat,
 DR> -        &nitems, &bytes_after, &buf)) {
 DR> +    prop = atomrep->atom;
 DR> +    free(atomrep);
 DR> +    proprep=xcb_get_property_reply(xd, xcb_get_property_unchecked(xd, FALSE, xcb_setup_roots_iterator(xcb_get_setup(xd)).data->root, prop, XCB_ATOM_ANY, 0, -1), NULL);
 DR> +    if (!proprep) {
 DR>          Errorx("No XFree86_VT property detected on X server, WINDOWPATH won't be set");
 DR>          return;
 DR>      }
 DR> -    if (nitems != 1) {
 DR> -        Errorx("XFree86_VT property unexpectedly has %lu items instead of 1", nitems);
 DR> -        XFree(buf);
 DR> +    actualformat = proprep->format;
 DR> +    actualtype = proprep->type;
 DR> +    bytes_after = proprep->bytes_after;
 DR> +    nitems = xcb_get_property_value_length(proprep);
 DR> +    if (nitems != actualformat/8) {
 DR> +        Errorx("XFree86_VT property unexpectedly has %lu items instead of 1", nitems / (actualformat/8));
 DR> +        free(proprep);
 DR>          return;
 DR>      }
 DR> +    buf = xcb_get_property_value(proprep);
 DR>      switch (actualtype) {
 DR> -    case XA_CARDINAL:
 DR> -    case XA_INTEGER:
 DR> -    case XA_WINDOW:
 DR> +    case XCB_ATOM_CARDINAL:
 DR> +    case XCB_ATOM_INTEGER:
 DR> +    case XCB_ATOM_WINDOW:
 DR>          switch (actualformat) {
 DR>          case  8:
 DR> -            num = (*(uint8_t  *)(void *)buf);
 DR> +            num = (*(uint8_t  *)buf);
 DR>              break;
 DR>          case 16:
 DR> -            num = (*(uint16_t *)(void *)buf);
 DR> +            num = (*(uint16_t *)buf);
 DR>              break;
 DR>          case 32:
 DR> -            num = (*(uint32_t *)(void *)buf);
 DR> +            num = (*(uint32_t *)buf);
 DR>              break;
 DR>          default:
 DR>              Errorx("XFree86_VT property has unexpected format %d", actualformat);
 DR> -            XFree(buf);
 DR> +            free(proprep);
 DR>              return;
 DR>          }
 DR>          break;
 DR>      default:
 DR>          Errorx("XFree86_VT property has unexpected type %lx", actualtype);
 DR> -        XFree(buf);
 DR> +        free(proprep);
 DR>          return;
 DR>      }
 DR> -    XFree(buf);
 DR> +    free(proprep);
 DR>      windowpath = getenv("WINDOWPATH");
 DR>      numn = snprintf(nums, sizeof(nums), "%lu", num);
 DR>      if (!windowpath) {
 DR> @@ -583,24 +588,12 @@ startClient(char *client[])
 DR>  
 DR>  static jmp_buf close_env;
 DR>  
 DR> -static int
 DR> -ignorexio(Display *dpy)
 DR> -{
 DR> -    Errorx("connection to X server lost");
 DR> -    longjmp(close_env, 1);
 DR> -    /*NOTREACHED*/
 DR> -    return 0;
 DR> -}
 DR> -
 DR>  static void
 DR>  shutdown(void)
 DR>  {
 DR>      /* have kept display opened, so close it now */
 DR>      if (clientpid > 0) {
 DR> -        XSetIOErrorHandler(ignorexio);
 DR> -        if (! setjmp(close_env)) {
 DR> -            XCloseDisplay(xd);
 DR> -        }
 DR> +        xcb_disconnect(xd);
 DR>  
 DR>          /* HUP all local clients to allow them to clean up */
 DR>          if (killpg(clientpid, SIGHUP) < 0 && errno != ESRCH)
 DR> -- 
 DR> 1.7.4.1

 DR> _______________________________________________
 DR> xorg-devel at lists.x.org: X.Org development
 DR> Archives: http://lists.x.org/archives/xorg-devel
 DR> Info: http://lists.x.org/mailman/listinfo/xorg-devel

-- 
  http://fossarchy.blogspot.com/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110311/f723239f/attachment.pgp>


More information about the xorg-devel mailing list