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

2008-11-19 Thread Alan Coopersmith 
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-   [EMAIL PROTECTED]
 Sun Microsystems, Inc. - X Window System Engineering

From ba5aed47f241ada8c73b61b3d5e18a1c2ccd268a Mon Sep 17 00:00:00 2001
From: Alan Coopersmith [EMAIL PROTECTED]
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);
 
 XkbEnsureSafeMapName(keymap);
 OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir));
-- 
1.5.6.5

___
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg

Re: [PATCH] Always use server-display.xkm for xkbcomp output files

2008-11-19 Thread Adam Jackson 
On Wed, 2008-11-19 at 14:04 -0800, Alan Coopersmith 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?

Hey, it deletes code, how can it not be good.

Signed-off-by: Adam Jackson [EMAIL PROTECTED]

- ajax


signature.asc
Description: This is a digitally signed message part
___
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg

Re: [PATCH] Always use server-display.xkm for xkbcomp output files

2008-11-19 Thread Dan Nicholson 
On Wed, Nov 19, 2008 at 2:04 PM, Alan Coopersmith
[EMAIL PROTECTED] 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-   [EMAIL PROTECTED]
 Sun Microsystems, Inc. - X Window System Engineering


 From ba5aed47f241ada8c73b61b3d5e18a1c2ccd268a Mon Sep 17 00:00:00 2001
 From: Alan Coopersmith [EMAIL PROTECTED]
 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
___
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg