[PATCH 1/2] Cache xkbcomp output

Keith Packard keithp at keithp.com
Fri Jul 13 13:08:34 PDT 2012


Construct a unique filename based on the display name and the RMLVO
values. If that file contains valid contents, use it. Otherwise,
compile the keymap to that file and don't unlink it so that it will be
re-used the next time the server runs.

v2: Only cache files that get written to XKM_OUTPUT_DIR.
    Add --disable-xkb-cache config option to not do any of this

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 configure.ac            |    8 ++
 include/xkb-config.h.in |    3 +
 include/xkbsrv.h        |    8 +-
 xkb/ddxLoad.c           |  200 +++++++++++++++++++++++++++++------------------
 xkb/xkb.c               |    5 +-
 5 files changed, 138 insertions(+), 86 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6456192..aa65159 100644
--- a/configure.ac
+++ b/configure.ac
@@ -519,6 +519,9 @@ AC_ARG_WITH(xkb-path,         AS_HELP_STRING([--with-xkb-path=PATH], [Path to XK
 AC_ARG_WITH(xkb-output,       AS_HELP_STRING([--with-xkb-output=PATH], [Path to XKB output dir (default: ${datadir}/X11/xkb/compiled)]),
 				[ XKBOUTPUT="$withval" ],
 				[ XKBOUTPUT="compiled" ])
+AC_ARG_ENABLE(xkb-cache,	AS_HELP_STRING([--disable-xkb-cache],
+					       [Cache xkbcomp output when possible (default: enabled)]),
+					       [XKBCACHE=$enableval], [XKBCACHE=yes])
 AC_ARG_WITH(default-xkb-rules, AS_HELP_STRING([--with-default-xkb-rules=RULES],
                                    [Keyboard ruleset (default: base/evdev)]),
                                 [ XKB_DFLT_RULES="$withval" ],
@@ -1227,6 +1230,11 @@ if [[ x$XKBOUTPUT_FIRSTCHAR != x/ -a x$XKBOUTPUT_FIRSTCHAR != 'x$' ]] ; then
    XKBOUTPUT="$XKB_BASE_DIRECTORY/$XKBOUTPUT"
 fi
 
+echo "XKBCACHE is $XKBCACHE"
+if test "x$XKBCACHE" = xyes; then
+	AC_DEFINE(XKM_CACHE_FILES, 1, [Cache xkbcomp output files])
+fi
+
 dnl XKM_OUTPUT_DIR (used in code) must end in / or file names get hosed
 dnl XKB_COMPILED_DIR (used in Makefiles) must not or install-sh gets confused
 
diff --git a/include/xkb-config.h.in b/include/xkb-config.h.in
index 7b6a671..94e0ed6 100644
--- a/include/xkb-config.h.in
+++ b/include/xkb-config.h.in
@@ -29,4 +29,7 @@
 /* XKB output dir for compiled keymaps. */
 #undef XKM_OUTPUT_DIR
 
+/* Cache .xkm files in XKM_OUTPUT_DIR */
+#undef XKM_CACHE_FILES
+
 #endif /* _XKB_CONFIG_H_ */
diff --git a/include/xkbsrv.h b/include/xkbsrv.h
index d584785..7b17282 100644
--- a/include/xkbsrv.h
+++ b/include/xkbsrv.h
@@ -857,12 +857,8 @@ extern _X_EXPORT unsigned int XkbDDXLoadKeymapByNames(DeviceIntPtr /* keybd */ ,
                                                       /* names */ ,
                                                       unsigned int /* want */ ,
                                                       unsigned int /* need */ ,
-                                                      XkbDescPtr *
-                                                      /* finfoRtrn */ ,
-                                                      char *
-                                                      /* keymapNameRtrn */ ,
-                                                      int       /* keymapNameRtrnLen */
-    );
+                                                      XkbDescPtr *xkbRtrn,
+                                                      char *path_name);
 
 extern _X_EXPORT Bool XkbDDXNamesFromRules(DeviceIntPtr /* keybd */ ,
                                            char * /* rules */ ,
diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index cb2dfc3..3258f88 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -144,24 +144,30 @@ Win32System(const char *cmdline)
 #endif
 
 static void
-OutputDirectory(char *outdir, size_t size)
+OutputDirectory(char *outdir, size_t size, Bool *is_private_directory)
 {
 #ifndef WIN32
     /* Can we write an xkm and then open it too? */
     if (access(XKM_OUTPUT_DIR, W_OK | X_OK) == 0 &&
         (strlen(XKM_OUTPUT_DIR) < size)) {
         (void) strcpy(outdir, XKM_OUTPUT_DIR);
+        if (is_private_directory)
+            *is_private_directory = TRUE;
     }
     else
 #else
     if (strlen(Win32TempDir()) + 1 < size) {
         (void) strcpy(outdir, Win32TempDir());
         (void) strcat(outdir, "\\");
+        if (is_private_directory)
+            *is_private_directory = FALSE;
     }
     else
 #endif
     if (strlen("/tmp/") < size) {
         (void) strcpy(outdir, "/tmp/");
+        if (is_private_directory)
+            *is_private_directory = FALSE;
     }
 }
 
@@ -169,10 +175,10 @@ static Bool
 XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
                            XkbComponentNamesPtr names,
                            unsigned want,
-                           unsigned need, char *nameRtrn, int nameRtrnLen)
+                           unsigned need, char *file_name)
 {
     FILE *out;
-    char *buf = NULL, keymap[PATH_MAX], xkm_output_dir[PATH_MAX];
+    char *buf = NULL;
 
     const char *emptystring = "";
     char *xkbbasedirflag = NULL;
@@ -188,10 +194,6 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
     const char *xkmfile = "-";
 #endif
 
-    snprintf(keymap, sizeof(keymap), "server-%s", display);
-
-    OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir));
-
 #ifdef WIN32
     strcpy(tmpname, Win32TempDir());
     strcat(tmpname, "\\xkb_XXXXXX");
@@ -216,13 +218,13 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
 
     if (asprintf(&buf,
                  "\"%s%sxkbcomp\" -w %d %s -xkm \"%s\" "
-                 "-em1 %s -emp %s -eml %s \"%s%s.xkm\"",
+                 "-em1 %s -emp %s -eml %s \"%s\"",
                  xkbbindir, xkbbindirsep,
                  ((xkbDebugFlags < 2) ? 1 :
                   ((xkbDebugFlags > 10) ? 10 : (int) xkbDebugFlags)),
                  xkbbasedirflag ? xkbbasedirflag : "", xkmfile,
                  PRE_ERROR_MSG, ERROR_PREFIX, POST_ERROR_MSG1,
-                 xkm_output_dir, keymap) == -1)
+                 file_name) == -1)
         buf = NULL;
 
     free(xkbbasedirflag);
@@ -255,9 +257,6 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
         {
             if (xkbDebugFlags)
                 DebugF("[xkb] xkb executes: %s\n", buf);
-            if (nameRtrn) {
-                strlcpy(nameRtrn, keymap, nameRtrnLen);
-            }
             free(buf);
 #ifdef WIN32
             unlink(tmpname);
@@ -265,7 +264,7 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
             return TRUE;
         }
         else
-            LogMessage(X_ERROR, "Error compiling keymap (%s)\n", keymap);
+            LogMessage(X_ERROR, "Error compiling keymap (%s)\n", file_name);
 #ifdef WIN32
         /* remove the temporary file */
         unlink(tmpname);
@@ -278,46 +277,58 @@ XkbDDXCompileKeymapByNames(XkbDescPtr xkb,
         LogMessage(X_ERROR, "Could not open file %s\n", tmpname);
 #endif
     }
-    if (nameRtrn)
-        nameRtrn[0] = '\0';
     free(buf);
     return FALSE;
 }
 
-static FILE *
-XkbDDXOpenConfigFile(char *mapName, char *fileNameRtrn, int fileNameRtrnLen)
+static char *
+xkb_config_pathname(char *filename, Bool *is_private_directory)
 {
-    char buf[PATH_MAX], xkm_output_dir[PATH_MAX];
-    FILE *file;
+    char        xkm_output_dir[PATH_MAX];
+    char        *pathname;
 
-    buf[0] = '\0';
-    if (mapName != NULL) {
-        OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir));
-        if ((XkbBaseDirectory != NULL) && (xkm_output_dir[0] != '/')
+    OutputDirectory(xkm_output_dir, sizeof (xkm_output_dir), is_private_directory);
+
+    if ((XkbBaseDirectory != NULL) && (xkm_output_dir[0] != '/')
 #ifdef WIN32
-            && (!isalpha(xkm_output_dir[0]) || xkm_output_dir[1] != ':')
+        && (!isalpha(xkm_output_dir[0]) || xkm_output_dir[1] != ':')
 #endif
-            ) {
-            if (snprintf(buf, PATH_MAX, "%s/%s%s.xkm", XkbBaseDirectory,
-                         xkm_output_dir, mapName) >= PATH_MAX)
-                buf[0] = '\0';
-        }
-        else {
-            if (snprintf(buf, PATH_MAX, "%s%s.xkm", xkm_output_dir, mapName)
-                >= PATH_MAX)
-                buf[0] = '\0';
-        }
-        if (buf[0] != '\0')
-            file = fopen(buf, "rb");
-        else
-            file = NULL;
+        ) {
+        if (asprintf (&pathname, "%s%s%s", XkbBaseDirectory,
+                      xkm_output_dir, filename) <= 0)
+            pathname = NULL;
     }
-    else
-        file = NULL;
-    if ((fileNameRtrn != NULL) && (fileNameRtrnLen > 0)) {
-        strlcpy(fileNameRtrn, buf, fileNameRtrnLen);
+    else {
+        if (asprintf (&pathname, "%s%s", xkm_output_dir, filename) <= 0)
+            pathname = NULL;
+    }
+    return pathname;
+}
+
+static unsigned
+xkb_load_keymap_file(char *file_name, unsigned want, unsigned need, XkbDescPtr *xkbRtrn)
+{
+    FILE *file;
+    unsigned missing;
+
+    file = fopen(file_name, "r");
+    if (file == NULL) {
+        LogMessage(X_ERROR, "Couldn't open compiled keymap file %s\n",
+                   file_name);
+        return 0;
+    }
+    missing = XkmReadFile(file, need, want, xkbRtrn);
+    if (*xkbRtrn == NULL) {
+        LogMessage(X_ERROR, "Error loading keymap %s\n", file_name);
+        fclose(file);
+        return 0;
+    }
+    else {
+        DebugF("Loaded XKB keymap %s, defined=0x%x\n", file_name,
+               (*xkbRtrn)->defined);
     }
-    return file;
+    fclose(file);
+    return (need | want) & (~missing);
 }
 
 unsigned
@@ -325,12 +336,21 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
                         XkbComponentNamesPtr names,
                         unsigned want,
                         unsigned need,
-                        XkbDescPtr *xkbRtrn, char *nameRtrn, int nameRtrnLen)
+                        XkbDescPtr *xkbRtrn, char *file_name)
 {
     XkbDescPtr xkb;
-    FILE *file;
-    char fileName[PATH_MAX];
-    unsigned missing;
+    unsigned provided;
+    char *local_file_name = file_name;
+
+    if (!file_name) {
+        char    local_name[PATH_MAX];
+
+        if (snprintf(local_name, sizeof (local_name), "server-%s.xkm", display) >= sizeof (local_name))
+            return 0;
+        local_file_name = xkb_config_pathname (local_name, NULL);
+        if (local_file_name == NULL)
+            return 0;
+    }
 
     *xkbRtrn = NULL;
     if ((keybd == NULL) || (keybd->key == NULL) ||
@@ -343,33 +363,31 @@ XkbDDXLoadKeymapByNames(DeviceIntPtr keybd,
         (names->geometry == NULL)) {
         LogMessage(X_ERROR, "XKB: No components provided for device %s\n",
                    keybd->name ? keybd->name : "(unnamed keyboard)");
+        if (local_file_name != file_name)
+            free(local_file_name);
         return 0;
     }
     else if (!XkbDDXCompileKeymapByNames(xkb, names, want, need,
-                                         nameRtrn, nameRtrnLen)) {
+                                         file_name)) {
         LogMessage(X_ERROR, "XKB: Couldn't compile keymap\n");
+        if (local_file_name != file_name)
+            free(local_file_name);
         return 0;
     }
-    file = XkbDDXOpenConfigFile(nameRtrn, fileName, PATH_MAX);
-    if (file == NULL) {
-        LogMessage(X_ERROR, "Couldn't open compiled keymap file %s\n",
-                   fileName);
-        return 0;
-    }
-    missing = XkmReadFile(file, need, want, xkbRtrn);
-    if (*xkbRtrn == NULL) {
-        LogMessage(X_ERROR, "Error loading keymap %s\n", fileName);
-        fclose(file);
-        (void) unlink(fileName);
+
+    provided = xkb_load_keymap_file(file_name, want, need, xkbRtrn);
+
+    if (!xkbRtrn) {
+        unlink(local_file_name);
+        if (local_file_name != file_name)
+            free(local_file_name);
         return 0;
     }
-    else {
-        DebugF("Loaded XKB keymap %s, defined=0x%x\n", fileName,
-               (*xkbRtrn)->defined);
+    if (local_file_name != file_name) {
+        unlink(local_file_name);
+        free(local_file_name);
     }
-    fclose(file);
-    (void) unlink(fileName);
-    return (need | want) & (~missing);
+    return provided;
 }
 
 Bool
@@ -445,22 +463,52 @@ static XkbDescPtr
 XkbCompileKeymapForDevice(DeviceIntPtr dev, XkbRMLVOSet * rmlvo, int need)
 {
     XkbDescPtr xkb = NULL;
-    unsigned int provided;
+    unsigned int provided = 0;
     XkbComponentNamesRec kccgst = { 0 };
-    char name[PATH_MAX];
-
-    if (XkbRMLVOtoKcCGST(dev, rmlvo, &kccgst)) {
-        provided =
-            XkbDDXLoadKeymapByNames(dev, &kccgst, XkmAllIndicesMask, need, &xkb,
-                                    name, PATH_MAX);
-        if ((need & provided) != need) {
-            if (xkb) {
-                XkbFreeKeyboard(xkb, 0, TRUE);
-                xkb = NULL;
+    char *filename, *pathname;
+    Bool is_private_directory;
+    Bool unlink_on_success = TRUE;
+
+    if (asprintf(&filename, "server-%s-%s-%s-%s-%s-%s.xkm",
+                 display,
+                 rmlvo->rules,
+                 rmlvo->model,
+                 rmlvo->layout,
+                 rmlvo->variant,
+                 rmlvo->options) <= 0)
+        return NULL;
+
+    pathname = xkb_config_pathname(filename, &is_private_directory);
+    if (pathname == NULL)
+        return NULL;
+
+#if XKM_CACHE_FILES
+    unlink_on_success = !is_private_directory;
+    if (is_private_directory)
+        provided = xkb_load_keymap_file(pathname, XkmAllIndicesMask, need, &xkb);
+#endif
+
+    if ((need & provided) != need) {
+        (void) unlink(pathname);
+        if (xkb)
+            XkbFreeKeyboard(xkb, 0, TRUE);
+        if (XkbRMLVOtoKcCGST(dev, rmlvo, &kccgst)) {
+            provided = XkbDDXLoadKeymapByNames(dev, &kccgst, XkmAllIndicesMask,
+                                               need, &xkb, pathname);
+            if ((need & provided) != need) {
+                if (xkb) {
+                    (void) unlink(pathname);
+                    XkbFreeKeyboard(xkb, 0, TRUE);
+                    xkb = NULL;
+                }
             }
         }
     }
 
+    if (unlink_on_success)
+        (void) unlink(pathname);
+    free (pathname);
+
     XkbFreeComponentNames(&kccgst, FALSE);
     return xkb;
 }
diff --git a/xkb/xkb.c b/xkb/xkb.c
index 4440a98..8b2684b 100644
--- a/xkb/xkb.c
+++ b/xkb/xkb.c
@@ -5704,7 +5704,6 @@ ProcXkbGetKbdByName(ClientPtr client)
     XkbComponentNamesRec names = { 0 };
     XkbDescPtr xkb, new;
     unsigned char *str;
-    char mapFile[PATH_MAX];
     unsigned len;
     unsigned fwant, fneed, reported;
     int status;
@@ -5762,7 +5761,6 @@ ProcXkbGetKbdByName(ClientPtr client)
         geom_changed = FALSE;
     }
 
-    memset(mapFile, 0, PATH_MAX);
     rep.type = X_Reply;
     rep.deviceID = dev->id;
     rep.sequenceNumber = client->sequence;
@@ -5784,8 +5782,7 @@ ProcXkbGetKbdByName(ClientPtr client)
     }
 
     /* We pass dev in here so we can get the old names out if needed. */
-    rep.found = XkbDDXLoadKeymapByNames(dev, &names, fwant, fneed, &new,
-                                        mapFile, PATH_MAX);
+    rep.found = XkbDDXLoadKeymapByNames(dev, &names, fwant, fneed, &new, NULL);
     rep.newKeyboard = FALSE;
     rep.pad1 = rep.pad2 = rep.pad3 = rep.pad4 = 0;
 
-- 
1.7.10.4



More information about the xorg-devel mailing list