[PATCH] Always use server-<display>.xkm for xkbcomp output files

[Dan Nicholson]

On Wed, Nov 19, 2008 at 2:04 PM, Alan Coopersmith
<Alan.Coopersmith at sun.com> wrote:
> The attached patch is code we've used in Xsun for years, and our Sun Ray
> people recently hit the same issue again in their Xorg 1.2-based Xnewt
> server port.   It simply changes the Xserver to always use a filename
> containing the display number for xkm output, so that if two servers are
> starting at once with the same keymap they don't overwrite or delete
> each other's xkm files.    While we're probably more likely to hit this
> on Sun Ray servers which can have hundreds of X servers running, I think
> it could also hit a single user machine with :0 & :1 starting on different
> vt's at the same time.
>
> (Of course, the oft-mentioned change of not forking xkbcomp to compile to
>  xkm would also solve this, but this is a much simpler short-term fix.)
>
> Does this look good to everyone?   Anyone know of a reason it would not
> be a good idea?
>
> --
>        -Alan Coopersmith-           alan.coopersmith at sun.com
>         Sun Microsystems, Inc. - X Window System Engineering
>
>
> From ba5aed47f241ada8c73b61b3d5e18a1c2ccd268a Mon Sep 17 00:00:00 2001
> From: Alan Coopersmith <alan.coopersmith at sun.com>
> Date: Wed, 19 Nov 2008 13:44:26 -0800
> Subject: [PATCH] Always use server-<display>.xkm to avoid races when multiple servers start
>
> Previously each server starting ran xkbcomp with the output set to
> <keymapname>.xkm, read it, then deleted it - which led to races if
> two servers were starting at the same time with the same keymap.
>
> Sun bug #6773816 Xorg uses the same xkm output file for compiled keymap file
>  <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6773816>
> ---
>  xkb/ddxLoad.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
> index fc49d99..80da963 100644
> --- a/xkb/ddxLoad.c
> +++ b/xkb/ddxLoad.c
> @@ -192,16 +192,8 @@ char       *buf = NULL, keymap[PATH_MAX],xkm_output_dir[PATH_MAX];
>  #ifdef WIN32
>  char tmpname[PATH_MAX];
>  #endif
> -    if ((names->keymap==NULL)||(names->keymap[0]=='\0')) {
> -       sprintf(keymap,"server-%s",display);
> -    }
> -    else {
> -       if (strlen(names->keymap) > PATH_MAX - 1) {
> -           ErrorF("[xkb] name of keymap (%s) exceeds max length\n", names->keymap);
> -           return False;
> -       }
> -       strcpy(keymap,names->keymap);
> -    }
> +
> +    snprintf(keymap, sizeof(keymap), "server-%s", display);

Well, I think Daniel removed the support for pre-built keymaps in
ab79110a, so ->keymap will always be NULL. It's effectively dead code,
anyway.

--
Dan