Commit efc2d74141fec07b0aa44640fbf667d6b52a8631

Ran Benita 2012-08-27T18:58:36

xkbcomp: clean up compile_keymap function We make the xkb_file_type enum sequential instead of masks, and then we don't have to repeat the file types several times in the function. Makes the code cleaner. Signed-off-by: Ran Benita <ran234@gmail.com>

diff --git a/src/text.c b/src/text.c
index 472dbcd..ae71d3a 100644
--- a/src/text.c
+++ b/src/text.c
@@ -226,25 +226,21 @@ ModMaskText(xkb_mod_mask_t mask)
     return buf;
 }
 
+static const char *xkb_file_type_strings[_FILE_TYPE_NUM_ENTRIES] = {
+    [FILE_TYPE_KEYMAP]   = "xkb_keymap",
+    [FILE_TYPE_TYPES]    = "xkb_types",
+    [FILE_TYPE_COMPAT]   = "xkb_compatibility",
+    [FILE_TYPE_SYMBOLS]  = "xkb_symbols",
+    [FILE_TYPE_KEYCODES] = "xkb_keycodes",
+    [FILE_TYPE_RULES]    = "rules",
+};
+
 const char *
 FileTypeText(enum xkb_file_type type)
 {
-    switch (type) {
-    case FILE_TYPE_KEYMAP:
-        return "xkb_keymap";
-    case FILE_TYPE_TYPES:
-        return "xkb_types";
-    case FILE_TYPE_COMPAT:
-        return "xkb_compatability";
-    case FILE_TYPE_SYMBOLS:
-        return "xkb_symbols";
-    case FILE_TYPE_KEYCODES:
-        return "xkb_keycodes";
-    case FILE_TYPE_RULES:
-        return "rules";
-    default:
+    if (type > _FILE_TYPE_NUM_ENTRIES)
         return "unknown";
-    }
+    return xkb_file_type_strings[type];
 }
 
 static const char *actionTypeNames[XkbSA_NumActions] = {
diff --git a/src/xkb-priv.h b/src/xkb-priv.h
index 1165812..dbdfe44 100644
--- a/src/xkb-priv.h
+++ b/src/xkb-priv.h
@@ -96,18 +96,25 @@ typedef uint32_t xkb_atom_t;
 #define XKB_LEVEL_INVALID 0xffffffff
 
 enum xkb_file_type {
-    /* The top level file which includes the other component files. */
-    FILE_TYPE_KEYMAP = (1 << 0),
+    /* Component files, by order of compilation. */
+    FILE_TYPE_KEYCODES = 0,
+    FILE_TYPE_TYPES = 1,
+    FILE_TYPE_COMPAT = 2,
+    FILE_TYPE_SYMBOLS = 3,
+    /* Geometry is not compiled any more. */
+    FILE_TYPE_GEOMETRY = 4,
 
-    /* Component files. */
-    FILE_TYPE_TYPES = (1 << 1),
-    FILE_TYPE_COMPAT = (1 << 2),
-    FILE_TYPE_SYMBOLS = (1 << 3),
-    FILE_TYPE_KEYCODES = (1 << 4),
-    FILE_TYPE_GEOMETRY = (1 << 5),
+    /* A top level file which includes the above files. */
+    FILE_TYPE_KEYMAP,
+
+/* File types which must be found in a keymap file. */
+#define FIRST_KEYMAP_FILE_TYPE FILE_TYPE_KEYCODES
+#define LAST_KEYMAP_FILE_TYPE  FILE_TYPE_SYMBOLS
 
     /* This one doesn't mix with the others, but useful here as well. */
-    FILE_TYPE_RULES = (1 << 6),
+    FILE_TYPE_RULES,
+
+    _FILE_TYPE_NUM_ENTRIES
 };
 
 struct xkb_context {
@@ -128,11 +135,6 @@ struct xkb_context {
     struct atom_table *atom_table;
 };
 
-/* Files needed for a complete keymap. */
-#define REQUIRED_FILE_TYPES (FILE_TYPE_TYPES | FILE_TYPE_COMPAT | \
-                             FILE_TYPE_SYMBOLS | FILE_TYPE_KEYCODES)
-#define LEGAL_FILE_TYPES    REQUIRED_FILE_TYPES
-
 /**
  * Legacy names for the components of an XKB keymap, also known as KcCGST.
  */
diff --git a/src/xkbcomp/ast-build.c b/src/xkbcomp/ast-build.c
index c4f6789..bbafaf4 100644
--- a/src/xkbcomp/ast-build.c
+++ b/src/xkbcomp/ast-build.c
@@ -489,7 +489,7 @@ XkbFileFromComponents(struct xkb_context *ctx,
                             (ParseCommon *) inc, 0);
     AppendStmt(&keycodes->common, &symbols->common);
 
-    return XkbFileCreate(ctx, FILE_TYPE_KEYMAP, strdup(""),
+    return XkbFileCreate(ctx, FILE_TYPE_KEYMAP, NULL,
                          &keycodes->common, 0);
 }
 
diff --git a/src/xkbcomp/include.c b/src/xkbcomp/include.c
index a7a8bbc..7d0004d 100644
--- a/src/xkbcomp/include.c
+++ b/src/xkbcomp/include.c
@@ -123,37 +123,25 @@ ParseIncludeMap(char **str_inout, char **file_rtrn, char **map_rtrn,
 
 /***====================================================================***/
 
+static const char *xkb_file_type_include_dirs[_FILE_TYPE_NUM_ENTRIES] = {
+    [FILE_TYPE_KEYCODES] = "keycodes",
+    [FILE_TYPE_TYPES] = "types",
+    [FILE_TYPE_COMPAT] = "compat",
+    [FILE_TYPE_SYMBOLS] = "symbols",
+    [FILE_TYPE_GEOMETRY] = "geometry",
+    [FILE_TYPE_KEYMAP] = "keymap",
+    [FILE_TYPE_RULES] = "rules",
+};
+
 /**
  * Return the xkb directory based on the type.
  */
 static const char *
 DirectoryForInclude(enum xkb_file_type type)
 {
-    switch (type) {
-    case FILE_TYPE_KEYMAP:
-        return "keymap";
-
-    case FILE_TYPE_KEYCODES:
-        return "keycodes";
-
-    case FILE_TYPE_TYPES:
-        return "types";
-
-    case FILE_TYPE_SYMBOLS:
-        return "symbols";
-
-    case FILE_TYPE_COMPAT:
-        return "compat";
-
-    case FILE_TYPE_GEOMETRY:
-        return "geometry";
-
-    case FILE_TYPE_RULES:
-        return "rules";
-
-    default:
+    if (type >= _FILE_TYPE_NUM_ENTRIES)
         return "";
-    }
+    return xkb_file_type_include_dirs[type];
 }
 
 /***====================================================================***/
diff --git a/src/xkbcomp/xkbcomp.c b/src/xkbcomp/xkbcomp.c
index a65550a..e0a1cfe 100644
--- a/src/xkbcomp/xkbcomp.c
+++ b/src/xkbcomp/xkbcomp.c
@@ -28,22 +28,25 @@
 #include "text.h"
 #include "rules.h"
 
-/**
- * Compile the given file and store the output in keymap.
- * @param file A list of XkbFiles, each denoting one type (e.g.
- * FILE_TYPE_KEYCODES, etc.)
- */
+typedef bool (*compile_file_fn)(XkbFile *file,
+                                struct xkb_keymap *keymap,
+                                enum merge_mode merge);
+
+static const compile_file_fn compile_file_fns[LAST_KEYMAP_FILE_TYPE + 1] = {
+    [FILE_TYPE_KEYCODES] = CompileKeycodes,
+    [FILE_TYPE_TYPES] = CompileKeyTypes,
+    [FILE_TYPE_COMPAT] = CompileCompatMap,
+    [FILE_TYPE_SYMBOLS] = CompileSymbols,
+};
+
 static struct xkb_keymap *
-compile_keymap(struct xkb_context *ctx, XkbFile *file)
+compile_keymap_file(struct xkb_context *ctx, XkbFile *file)
 {
     bool ok;
-    unsigned have = 0;
     const char *main_name;
     struct xkb_keymap *keymap;
-    XkbFile *keycodes = NULL;
-    XkbFile *types = NULL;
-    XkbFile *compat = NULL;
-    XkbFile *symbols = NULL;
+    XkbFile *files[LAST_KEYMAP_FILE_TYPE + 1] = { NULL };
+    enum xkb_file_type type;
 
     keymap = xkb_map_new(ctx);
     if (!keymap)
@@ -51,46 +54,26 @@ compile_keymap(struct xkb_context *ctx, XkbFile *file)
 
     main_name = file->name ? file->name : "(unnamed)";
 
-    /*
-     * Other aggregate file types are converted to FILE_TYPE_KEYMAP
-     * in the parser.
-     */
     if (file->file_type != FILE_TYPE_KEYMAP) {
         log_err(ctx, "Cannot compile a %s file alone into a keymap\n",
                 FileTypeText(file->file_type));
         goto err;
     }
 
-    /* Check for duplicate entries in the input file */
+    /* Collect section files and check for duplicates. */
     for (file = (XkbFile *) file->defs; file;
          file = (XkbFile *) file->common.next) {
-        if (have & file->file_type) {
-            log_err(ctx,
-                    "More than one %s section in a keymap file; "
-                    "All sections after the first ignored\n",
+        if (file->file_type < FIRST_KEYMAP_FILE_TYPE ||
+            file->file_type > LAST_KEYMAP_FILE_TYPE) {
+            log_err(ctx, "Cannot define %s in a keymap file\n",
                     FileTypeText(file->file_type));
             continue;
         }
 
-        switch (file->file_type) {
-        case FILE_TYPE_KEYCODES:
-            keycodes = file;
-            break;
-
-        case FILE_TYPE_TYPES:
-            types = file;
-            break;
-
-        case FILE_TYPE_SYMBOLS:
-            symbols = file;
-            break;
-
-        case FILE_TYPE_COMPAT:
-            compat = file;
-            break;
-
-        default:
-            log_err(ctx, "Cannot define %s in a keymap file\n",
+        if (files[file->file_type]) {
+            log_err(ctx,
+                    "More than one %s section in keymap file; "
+                    "All sections after the first ignored\n",
                     FileTypeText(file->file_type));
             continue;
         }
@@ -100,45 +83,37 @@ compile_keymap(struct xkb_context *ctx, XkbFile *file)
             file->topName = strdup(main_name);
         }
 
-        have |= file->file_type;
+        files[file->file_type] = file;
     }
 
-    if (REQUIRED_FILE_TYPES & (~have)) {
-        enum xkb_file_type bit;
-        enum xkb_file_type missing;
-
-        missing = REQUIRED_FILE_TYPES & (~have);
-
-        for (bit = 1; missing != 0; bit <<= 1) {
-            if (missing & bit) {
-                log_err(ctx, "Required section %s missing from keymap\n",
-                        FileTypeText(bit));
-                missing &= ~bit;
-            }
+    /*
+     * Check that all required section were provided.
+     * Report everything before failing.
+     */
+    ok = true;
+    for (type = FIRST_KEYMAP_FILE_TYPE;
+         type <= LAST_KEYMAP_FILE_TYPE;
+         type++) {
+        if (files[type] == NULL) {
+            log_err(ctx, "Required section %s missing from keymap\n",
+                    FileTypeText(type));
+            ok = false;
         }
-
-        goto err;
     }
-
-    ok = CompileKeycodes(keycodes, keymap, MERGE_OVERRIDE);
-    if (!ok) {
-        log_err(ctx, "Failed to compile keycodes\n");
-        goto err;
-    }
-    ok = CompileKeyTypes(types, keymap, MERGE_OVERRIDE);
-    if (!ok) {
-        log_err(ctx, "Failed to compile key types\n");
-        goto err;
-    }
-    ok = CompileCompatMap(compat, keymap, MERGE_OVERRIDE);
-    if (!ok) {
-        log_err(ctx, "Failed to compile compat map\n");
-        goto err;
-    }
-    ok = CompileSymbols(symbols, keymap, MERGE_OVERRIDE);
-    if (!ok) {
-        log_err(ctx, "Failed to compile symbols\n");
+    if (!ok)
         goto err;
+
+    /* Compile sections. */
+    for (type = FIRST_KEYMAP_FILE_TYPE;
+         type <= LAST_KEYMAP_FILE_TYPE;
+         type++) {
+        log_dbg(ctx, "Compiling %s \"%s\"\n",
+                FileTypeText(type), files[type]->topName);
+        ok = compile_file_fns[type](files[type], keymap, MERGE_OVERRIDE);
+        if (!ok) {
+            log_err(ctx, "Failed to compile %s\n", FileTypeText(type));
+            goto err;
+        }
     }
 
     ok = UpdateModifiersFromCompat(keymap);
@@ -194,7 +169,7 @@ xkb_map_new_from_names(struct xkb_context *ctx,
         return NULL;
     }
 
-    keymap = compile_keymap(ctx, file);
+    keymap = compile_keymap_file(ctx, file);
     FreeXkbFile(file);
     return keymap;
 }
@@ -225,7 +200,7 @@ xkb_map_new_from_string(struct xkb_context *ctx,
         return NULL;
     }
 
-    keymap = compile_keymap(ctx, file);
+    keymap = compile_keymap_file(ctx, file);
     FreeXkbFile(file);
     return keymap;
 }
@@ -256,7 +231,7 @@ xkb_map_new_from_file(struct xkb_context *ctx,
         return NULL;
     }
 
-    keymap = compile_keymap(ctx, xkb_file);
+    keymap = compile_keymap_file(ctx, xkb_file);
     FreeXkbFile(xkb_file);
     return keymap;
 }