xkbcomp: handle XKB file include's better

The 'merge_mode' situation is quite messy, and we've introduced a
regression compared to original xkbcomp: when handling a composite
include statement, such as
    replace "foo(bar)+baz(bla)|doo:dee"
and merging the entire resulting *Info back into the including *Info,
we actually use the merge mode that is set by the last part (here it is
"augment" because of the '|'), when we should be using the one set for
the whole statement (here "replace").

We also take the opportunity to clean up a bit.

Signed-off-by: Ran Benita <ran234@gmail.com>

diff --git a/src/xkbcomp/compat.c b/src/xkbcomp/compat.c
index 50e8801..cbc4b71 100644 (file)
--- a/src/xkbcomp/compat.c
+++ b/src/xkbcomp/compat.c
@@ -586,44 +586,42 @@ static void
 HandleCompatMapFile(CompatInfo *info, XkbFile *file, enum merge_mode merge);
 
 static bool
-HandleIncludeCompatMap(CompatInfo *info, IncludeStmt *stmt)
+HandleIncludeCompatMap(CompatInfo *info, IncludeStmt *include)
 {
-    enum merge_mode merge = MERGE_DEFAULT;
-    XkbFile *rtrn;
-    CompatInfo included, next_incl;
+    CompatInfo included;
 
     InitCompatInfo(&included, info->keymap, info->file_id, info->actions);
-    if (stmt->stmt) {
-        free(included.name);
-        included.name = stmt->stmt;
-        stmt->stmt = NULL;
-    }
+    included.name = include->stmt;
+    include->stmt = NULL;
+
+    for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) {
+        CompatInfo next_incl;
+        XkbFile *file;
 
-    for (; stmt; stmt = stmt->next_incl) {
-        if (!ProcessIncludeFile(info->keymap->ctx, stmt, FILE_TYPE_COMPAT,
-                                &rtrn, &merge)) {
+        file = ProcessIncludeFile(info->keymap->ctx, stmt, FILE_TYPE_COMPAT);
+        if (!file) {
             info->errorCount += 10;
             ClearCompatInfo(&included);
             return false;
         }
 
-        InitCompatInfo(&next_incl, info->keymap, rtrn->id, info->actions);
-        next_incl.file_id = rtrn->id;
+        InitCompatInfo(&next_incl, info->keymap, file->id, info->actions);
         next_incl.default_interp = info->default_interp;
-        next_incl.default_interp.file_id = rtrn->id;
-        next_incl.default_interp.merge = merge;
-        next_incl.default_led.file_id = rtrn->id;
-        next_incl.default_led.merge = merge;
+        next_incl.default_interp.file_id = file->id;
+        next_incl.default_interp.merge = stmt->merge;
+        next_incl.default_led = info->default_led;
+        next_incl.default_led.file_id = file->id;
+        next_incl.default_led.merge = stmt->merge;
 
-        HandleCompatMapFile(&next_incl, rtrn, MERGE_OVERRIDE);
+        HandleCompatMapFile(&next_incl, file, MERGE_OVERRIDE);
 
-        MergeIncludedCompatMaps(&included, &next_incl, merge);
+        MergeIncludedCompatMaps(&included, &next_incl, stmt->merge);
 
         ClearCompatInfo(&next_incl);
-        FreeXkbFile(rtrn);
+        FreeXkbFile(file);
     }
 
-    MergeIncludedCompatMaps(info, &included, merge);
+    MergeIncludedCompatMaps(info, &included, include->merge);
     ClearCompatInfo(&included);
 
     return (info->errorCount == 0);

diff --git a/src/xkbcomp/include.c b/src/xkbcomp/include.c
index b94ffd0..b4a4014 100644 (file)
--- a/src/xkbcomp/include.c
+++ b/src/xkbcomp/include.c
@@ -172,8 +172,6 @@ ParseIncludeMap(char **str_inout, char **file_rtrn, char **map_rtrn,
     return true;
 }
 
-/***====================================================================***/
-
 static const char *xkb_file_type_include_dirs[_FILE_TYPE_NUM_ENTRIES] = {
     [FILE_TYPE_KEYCODES] = "keycodes",
     [FILE_TYPE_TYPES] = "types",
@@ -195,19 +193,6 @@ DirectoryForInclude(enum xkb_file_type type)
     return xkb_file_type_include_dirs[type];
 }
 
-/***====================================================================***/
-
-/**
- * Search for the given file name in the include directories.
- *
- * @param ctx the XKB ctx containing the include paths
- * @param type one of FILE_TYPE_TYPES, FILE_TYPE_COMPAT, ..., or
- *             FILE_TYPE_KEYMAP or FILE_TYPE_RULES
- * @param pathRtrn is set to the full path of the file if found.
- *
- * @return an FD to the file or NULL. If NULL is returned, the value of
- * pathRtrn is undefined.
- */
 FILE *
 FindFileInXkbPath(struct xkb_context *ctx, const char *name,
                   enum xkb_file_type type, char **pathRtrn)
@@ -265,24 +250,9 @@ FindFileInXkbPath(struct xkb_context *ctx, const char *name,
     return file;
 }
 
-/**
- * Open the file given in the include statement and parse it's content.
- * If the statement defines a specific map to use, this map is returned in
- * file_rtrn. Otherwise, the default map is returned.
- *
- * @param ctx The ctx containing include paths
- * @param stmt The include statement, specifying the file name to look for.
- * @param file_type Type of file (FILE_TYPE_KEYCODES, etc.)
- * @param file_rtrn Returns the key map to be used.
- * @param merge_rtrn Always returns stmt->merge.
- *
- * @return true on success or false otherwise.
- */
-bool
-ProcessIncludeFile(struct xkb_context *ctx,
-                   IncludeStmt * stmt,
-                   enum xkb_file_type file_type,
-                   XkbFile ** file_rtrn, enum merge_mode *merge_rtrn)
+XkbFile *
+ProcessIncludeFile(struct xkb_context *ctx, IncludeStmt *stmt,
+                   enum xkb_file_type file_type)
 {
     FILE *file;
     XkbFile *xkb_file;
@@ -292,6 +262,7 @@ ProcessIncludeFile(struct xkb_context *ctx,
         return false;
 
     xkb_file = XkbParseFile(ctx, file, stmt->file, stmt->map);
+    fclose(file);
     if (!xkb_file) {
         if (stmt->map)
             log_err(ctx, "Couldn't process include statement for '%s(%s)'\n",
@@ -299,10 +270,8 @@ ProcessIncludeFile(struct xkb_context *ctx,
         else
             log_err(ctx, "Couldn't process include statement for '%s'\n",
                     stmt->file);
-        fclose(file);
-        return false;
+        return NULL;
     }
-    fclose(file);
 
     if (xkb_file->file_type != file_type) {
         log_err(ctx,
@@ -310,12 +279,11 @@ ProcessIncludeFile(struct xkb_context *ctx,
                 "Include file \"%s\" ignored\n",
                 xkb_file_type_to_string(file_type),
                 xkb_file_type_to_string(xkb_file->file_type), stmt->file);
-        return false;
+        FreeXkbFile(xkb_file);
+        return NULL;
     }
 
     /* FIXME: we have to check recursive includes here (or somewhere) */
 
-    *file_rtrn = xkb_file;
-    *merge_rtrn = stmt->merge;
-    return true;
+    return xkb_file;
 }

diff --git a/src/xkbcomp/include.h b/src/xkbcomp/include.h
index 9ba0b55..03e76ed 100644 (file)
--- a/src/xkbcomp/include.h
+++ b/src/xkbcomp/include.h
@@ -35,9 +35,8 @@ FILE *
 FindFileInXkbPath(struct xkb_context *ctx, const char *name,
                   enum xkb_file_type type, char **pathRtrn);
 
-bool
+XkbFile *
 ProcessIncludeFile(struct xkb_context *ctx, IncludeStmt *stmt,
-                   enum xkb_file_type file_type, XkbFile **file_rtrn,
-                   enum merge_mode *merge_rtrn);
+                   enum xkb_file_type file_type);
 
 #endif

diff --git a/src/xkbcomp/keycodes.c b/src/xkbcomp/keycodes.c
index 47bfecd..ae17caf 100644 (file)
--- a/src/xkbcomp/keycodes.c
+++ b/src/xkbcomp/keycodes.c
@@ -423,38 +423,36 @@ static void
 HandleKeycodesFile(KeyNamesInfo *info, XkbFile *file, enum merge_mode merge);
 
 static bool
-HandleIncludeKeycodes(KeyNamesInfo *info, IncludeStmt *stmt)
+HandleIncludeKeycodes(KeyNamesInfo *info, IncludeStmt *include)
 {
-    enum merge_mode merge = MERGE_DEFAULT;
-    XkbFile *rtrn;
-    KeyNamesInfo included, next_incl;
+    KeyNamesInfo included;
 
     InitKeyNamesInfo(&included, info->ctx, info->file_id);
-    if (stmt->stmt) {
-        free(included.name);
-        included.name = stmt->stmt;
-        stmt->stmt = NULL;
-    }
+    included.name = include->stmt;
+    include->stmt = NULL;
+
+    for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) {
+        KeyNamesInfo next_incl;
+        XkbFile *file;
 
-    for (; stmt; stmt = stmt->next_incl) {
-        if (!ProcessIncludeFile(info->ctx, stmt, FILE_TYPE_KEYCODES,
-                                &rtrn, &merge)) {
+        file = ProcessIncludeFile(info->ctx, stmt, FILE_TYPE_KEYCODES);
+        if (!file) {
             info->errorCount += 10;
             ClearKeyNamesInfo(&included);
             return false;
         }
 
-        InitKeyNamesInfo(&next_incl, info->ctx, rtrn->id);
+        InitKeyNamesInfo(&next_incl, info->ctx, file->id);
 
-        HandleKeycodesFile(&next_incl, rtrn, MERGE_OVERRIDE);
+        HandleKeycodesFile(&next_incl, file, MERGE_OVERRIDE);
 
-        MergeIncludedKeycodes(&included, &next_incl, merge);
+        MergeIncludedKeycodes(&included, &next_incl, stmt->merge);
 
         ClearKeyNamesInfo(&next_incl);
-        FreeXkbFile(rtrn);
+        FreeXkbFile(file);
     }
 
-    MergeIncludedKeycodes(info, &included, merge);
+    MergeIncludedKeycodes(info, &included, include->merge);
     ClearKeyNamesInfo(&included);
 
     return (info->errorCount == 0);

diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index b71fb09..c0f8cde 100644 (file)
--- a/src/xkbcomp/symbols.c
+++ b/src/xkbcomp/symbols.c
@@ -536,29 +536,26 @@ static void
 HandleSymbolsFile(SymbolsInfo *info, XkbFile *file, enum merge_mode merge);
 
 static bool
-HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *stmt)
+HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *include)
 {
-    enum merge_mode merge = MERGE_DEFAULT;
-    XkbFile *rtrn;
-    SymbolsInfo included, next_incl;
+    SymbolsInfo included;
 
     InitSymbolsInfo(&included, info->keymap, info->file_id, info->actions);
-    if (stmt->stmt) {
-        free(included.name);
-        included.name = stmt->stmt;
-        stmt->stmt = NULL;
-    }
+    included.name = include->stmt;
+    include->stmt = NULL;
+
+    for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) {
+        SymbolsInfo next_incl;
+        XkbFile *file;
 
-    for (; stmt; stmt = stmt->next_incl) {
-        if (!ProcessIncludeFile(info->keymap->ctx, stmt, FILE_TYPE_SYMBOLS,
-                                &rtrn, &merge)) {
+        file = ProcessIncludeFile(info->keymap->ctx, stmt, FILE_TYPE_SYMBOLS);
+        if (!file) {
             info->errorCount += 10;
             ClearSymbolsInfo(&included);
             return false;
         }
 
-        InitSymbolsInfo(&next_incl, info->keymap, rtrn->id, info->actions);
-        next_incl.merge = next_incl.default_key.merge = MERGE_OVERRIDE;
+        InitSymbolsInfo(&next_incl, info->keymap, file->id, info->actions);
         if (stmt->modifier) {
             next_incl.explicit_group = atoi(stmt->modifier) - 1;
             if (next_incl.explicit_group >= XKB_MAX_GROUPS) {
@@ -573,15 +570,15 @@ HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *stmt)
             next_incl.explicit_group = info->explicit_group;
         }
 
-        HandleSymbolsFile(&next_incl, rtrn, MERGE_OVERRIDE);
+        HandleSymbolsFile(&next_incl, file, MERGE_OVERRIDE);
 
-        MergeIncludedSymbols(&included, &next_incl, merge);
+        MergeIncludedSymbols(&included, &next_incl, stmt->merge);
 
         ClearSymbolsInfo(&next_incl);
-        FreeXkbFile(rtrn);
+        FreeXkbFile(file);
     }
 
-    MergeIncludedSymbols(info, &included, merge);
+    MergeIncludedSymbols(info, &included, include->merge);
     ClearSymbolsInfo(&included);
 
     return (info->errorCount == 0);

diff --git a/src/xkbcomp/types.c b/src/xkbcomp/types.c
index 2f019e3..737fc1e 100644 (file)
--- a/src/xkbcomp/types.c
+++ b/src/xkbcomp/types.c
@@ -314,38 +314,36 @@ static void
 HandleKeyTypesFile(KeyTypesInfo *info, XkbFile *file, enum merge_mode merge);
 
 static bool
-HandleIncludeKeyTypes(KeyTypesInfo *info, IncludeStmt *stmt)
+HandleIncludeKeyTypes(KeyTypesInfo *info, IncludeStmt *include)
 {
-    enum merge_mode merge = MERGE_DEFAULT;
-    XkbFile *rtrn;
-    KeyTypesInfo included, next_incl;
+    KeyTypesInfo included;
 
     InitKeyTypesInfo(&included, info->keymap, info->file_id);
-    if (stmt->stmt) {
-        free(included.name);
-        included.name = stmt->stmt;
-        stmt->stmt = NULL;
-    }
+    included.name = include->stmt;
+    include->stmt = NULL;
+
+    for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) {
+        KeyTypesInfo next_incl;
+        XkbFile *file;
 
-    for (; stmt; stmt = stmt->next_incl) {
-        if (!ProcessIncludeFile(info->keymap->ctx, stmt, FILE_TYPE_TYPES,
-                                &rtrn, &merge)) {
+        file = ProcessIncludeFile(info->keymap->ctx, stmt, FILE_TYPE_TYPES);
+        if (!file) {
             info->errorCount += 10;
             ClearKeyTypesInfo(&included);
             return false;
         }
 
-        InitKeyTypesInfo(&next_incl, info->keymap, rtrn->id);
+        InitKeyTypesInfo(&next_incl, info->keymap, file->id);
 
-        HandleKeyTypesFile(&next_incl, rtrn, merge);
+        HandleKeyTypesFile(&next_incl, file, stmt->merge);
 
-        MergeIncludedKeyTypes(&included, &next_incl, merge);
+        MergeIncludedKeyTypes(&included, &next_incl, stmt->merge);
 
         ClearKeyTypesInfo(&next_incl);
-        FreeXkbFile(rtrn);
+        FreeXkbFile(file);
     }
 
-    MergeIncludedKeyTypes(info, &included, merge);
+    MergeIncludedKeyTypes(info, &included, include->merge);
     ClearKeyTypesInfo(&included);
 
     return (info->errorCount == 0);