Refactor Compile<component> functions

The error handling was not ideal, so unify it. Also makes the functions
a bit easier to read.

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

diff --git a/src/xkbcomp/compat.c b/src/xkbcomp/compat.c
index 18d1615..0674ef0 100644 (file)
--- a/src/xkbcomp/compat.c
+++ b/src/xkbcomp/compat.c
@@ -791,7 +791,7 @@ CopyInterps(CompatInfo * info,
 }
 
 bool
-CompileCompatMap(XkbFile *file, struct xkb_keymap * xkb, unsigned merge,
+CompileCompatMap(XkbFile *file, struct xkb_keymap *xkb, unsigned merge,
                  LEDInfoPtr *unboundLEDs)
 {
     int i;
@@ -801,50 +801,47 @@ CompileCompatMap(XkbFile *file, struct xkb_keymap * xkb, unsigned merge,
     InitCompatInfo(&info, xkb);
     info.dflt.defs.merge = merge;
     info.ledDflt.defs.merge = merge;
+
     HandleCompatMapFile(file, xkb, merge, &info);
 
-    if (info.errorCount == 0)
-    {
-        int size;
-        if (XkbcAllocCompatMap(xkb, info.nInterps) != Success)
-        {
-            WSGO("Couldn't allocate compatibility map\n");
-            return false;
-        }
-        size = info.nInterps * sizeof(struct xkb_sym_interpret);
-        if (size > 0)
-        {
-            CopyInterps(&info, xkb->compat, true, XkbSI_Exactly);
-            CopyInterps(&info, xkb->compat, true, XkbSI_AllOf | XkbSI_NoneOf);
-            CopyInterps(&info, xkb->compat, true, XkbSI_AnyOf);
-            CopyInterps(&info, xkb->compat, true, XkbSI_AnyOfOrNone);
-            CopyInterps(&info, xkb->compat, false, XkbSI_Exactly);
-            CopyInterps(&info, xkb->compat, false,
-                        XkbSI_AllOf | XkbSI_NoneOf);
-            CopyInterps(&info, xkb->compat, false, XkbSI_AnyOf);
-            CopyInterps(&info, xkb->compat, false, XkbSI_AnyOfOrNone);
-        }
-        for (i = 0, gcm = &info.groupCompat[0]; i < XkbNumKbdGroups;
-             i++, gcm++)
-        {
-            if ((gcm->fileID != 0) || (gcm->real_mods != 0)
-                || (gcm->vmods != 0))
-            {
-                xkb->compat->groups[i].mask = gcm->real_mods;
-                xkb->compat->groups[i].real_mods = gcm->real_mods;
-                xkb->compat->groups[i].vmods = gcm->vmods;
-            }
-        }
-        if (info.leds != NULL)
-        {
-            if (!CopyIndicatorMapDefs(xkb, info.leds, unboundLEDs))
-                info.errorCount++;
-            info.leds = NULL;
+    if (info.errorCount != 0)
+        goto err_info;
+
+    if (XkbcAllocCompatMap(xkb, info.nInterps) != Success) {
+        WSGO("Couldn't allocate compatibility map\n");
+        goto err_info;
+    }
+
+    if (info.nInterps > 0) {
+        CopyInterps(&info, xkb->compat, true, XkbSI_Exactly);
+        CopyInterps(&info, xkb->compat, true, XkbSI_AllOf | XkbSI_NoneOf);
+        CopyInterps(&info, xkb->compat, true, XkbSI_AnyOf);
+        CopyInterps(&info, xkb->compat, true, XkbSI_AnyOfOrNone);
+        CopyInterps(&info, xkb->compat, false, XkbSI_Exactly);
+        CopyInterps(&info, xkb->compat, false, XkbSI_AllOf | XkbSI_NoneOf);
+        CopyInterps(&info, xkb->compat, false, XkbSI_AnyOf);
+        CopyInterps(&info, xkb->compat, false, XkbSI_AnyOfOrNone);
+    }
+
+    for (i = 0, gcm = &info.groupCompat[0]; i < XkbNumKbdGroups; i++, gcm++) {
+        if ((gcm->fileID != 0) || (gcm->real_mods != 0) || (gcm->vmods != 0)) {
+            xkb->compat->groups[i].mask = gcm->real_mods;
+            xkb->compat->groups[i].real_mods = gcm->real_mods;
+            xkb->compat->groups[i].vmods = gcm->vmods;
         }
-        ClearCompatInfo(&info, xkb);
-        return true;
     }
-    free(info.interps);
+
+    if (info.leds != NULL) {
+        if (!CopyIndicatorMapDefs(xkb, info.leds, unboundLEDs))
+            info.errorCount++;
+        info.leds = NULL;
+    }
+
+    ClearCompatInfo(&info, xkb);
+    return true;
+
+err_info:
+    ClearCompatInfo(&info, xkb);
     return false;
 }
 

diff --git a/src/xkbcomp/keycodes.c b/src/xkbcomp/keycodes.c
index 4715645..e768d91 100644 (file)
--- a/src/xkbcomp/keycodes.c
+++ b/src/xkbcomp/keycodes.c
@@ -874,57 +874,58 @@ HandleKeycodesFile(XkbFile * file,
  * @return true on success, false otherwise.
  */
 bool
-CompileKeycodes(XkbFile *file, struct xkb_keymap * xkb, unsigned merge)
+CompileKeycodes(XkbFile *file, struct xkb_keymap *xkb, unsigned merge)
 {
     KeyNamesInfo info; /* contains all the info after parsing */
 
     InitKeyNamesInfo(&info);
+
     HandleKeycodesFile(file, xkb, merge, &info);
 
     /* all the keys are now stored in info */
 
-    if (info.errorCount == 0)
-    {
-        if (info.explicitMin > 0) /* if "minimum" statement was present */
-            xkb->min_key_code = info.explicitMin;
-        else
-            xkb->min_key_code = info.computedMin;
-        if (info.explicitMax > 0) /* if "maximum" statement was present */
-            xkb->max_key_code = info.explicitMax;
-        else
-            xkb->max_key_code = info.computedMax;
-        if (XkbcAllocNames(xkb, XkbKeyNamesMask | XkbIndicatorNamesMask, 0)
-                == Success)
-        {
-            uint64_t i;
-            for (i = info.computedMin; i <= info.computedMax; i++)
-                LongToKeyName(info.names[i], xkb->names->keys[i].name);
-        }
-        else
-        {
-            WSGO("Cannot create struct xkb_names in CompileKeycodes\n");
-            return false;
+    if (info.errorCount != 0)
+        goto err_info;
+
+    if (info.explicitMin > 0) /* if "minimum" statement was present */
+        xkb->min_key_code = info.explicitMin;
+    else
+        xkb->min_key_code = info.computedMin;
+
+    if (info.explicitMax > 0) /* if "maximum" statement was present */
+        xkb->max_key_code = info.explicitMax;
+    else
+        xkb->max_key_code = info.computedMax;
+
+    if (XkbcAllocNames(xkb, XkbKeyNamesMask | XkbIndicatorNamesMask, 0)
+        == Success) {
+        uint64_t i;
+        for (i = info.computedMin; i <= info.computedMax; i++)
+            LongToKeyName(info.names[i], xkb->names->keys[i].name);
+    } else {
+        WSGO("Cannot create struct xkb_names in CompileKeycodes\n");
+        goto err_info;
+    }
+
+    if (info.leds) {
+        IndicatorNameInfo *ii;
+        if (XkbcAllocIndicatorMaps(xkb) != Success) {
+            WSGO("Couldn't allocate IndicatorRec in CompileKeycodes\n");
+            ACTION("Physical indicators not set\n");
         }
-        if (info.leds)
-        {
-            IndicatorNameInfo *ii;
-            if (XkbcAllocIndicatorMaps(xkb) != Success)
-            {
-                WSGO("Couldn't allocate IndicatorRec in CompileKeycodes\n");
-                ACTION("Physical indicators not set\n");
-            }
-            for (ii = info.leds; ii != NULL;
-                 ii = (IndicatorNameInfo *) ii->defs.next)
-            {
-                free(UNCONSTIFY(xkb->names->indicators[ii->ndx - 1]));
-                xkb->names->indicators[ii->ndx - 1] = XkbcAtomGetString(ii->name);
-            }
+
+        for (ii = info.leds; ii; ii = (IndicatorNameInfo *)ii->defs.next) {
+            free(UNCONSTIFY(xkb->names->indicators[ii->ndx - 1]));
+            xkb->names->indicators[ii->ndx - 1] = XkbcAtomGetString(ii->name);
         }
-        if (info.aliases)
-            ApplyAliases(xkb, &info.aliases);
-        ClearKeyNamesInfo(&info);
-        return true;
     }
+
+    ApplyAliases(xkb, &info.aliases);
+
+    ClearKeyNamesInfo(&info);
+    return true;
+
+err_info:
     ClearKeyNamesInfo(&info);
     return false;
 }

diff --git a/src/xkbcomp/keytypes.c b/src/xkbcomp/keytypes.c
index 2cc1366..03c5b04 100644 (file)
--- a/src/xkbcomp/keytypes.c
+++ b/src/xkbcomp/keytypes.c
@@ -1142,83 +1142,86 @@ CopyDefToKeyType(struct xkb_keymap * xkb, struct xkb_key_type * type, KeyTypeInf
 bool
 CompileKeyTypes(XkbFile *file, struct xkb_keymap * xkb, unsigned merge)
 {
+    unsigned int i;
+    struct xkb_key_type *type, *next;
     KeyTypesInfo info;
+    KeyTypeInfo *def;
 
     InitKeyTypesInfo(&info, xkb, NULL);
     info.fileID = file->id;
+
     HandleKeyTypesFile(file, xkb, merge, &info);
 
-    if (info.errorCount == 0)
-    {
-        unsigned int i;
-        KeyTypeInfo *def;
-        struct xkb_key_type *type, *next;
-
-        i = info.nTypes;
-        if ((info.stdPresent & XkbOneLevelMask) == 0)
-            i++;
-        if ((info.stdPresent & XkbTwoLevelMask) == 0)
-            i++;
-        if ((info.stdPresent & XkbKeypadMask) == 0)
-            i++;
-        if ((info.stdPresent & XkbAlphabeticMask) == 0)
-            i++;
-        if (XkbcAllocClientMap(xkb, XkbKeyTypesMask, i) != Success)
-        {
-            FreeKeyTypesInfo(&info);
-            WSGO("Couldn't allocate client map\n");
-            return false;
-        }
-        xkb->map->num_types = i;
-        if (XkbAllRequiredTypes & (~info.stdPresent))
-        {
-            unsigned missing, keypadVMod;
+    if (info.errorCount != 0)
+        goto err_info;
+
+    i = info.nTypes;
+    if ((info.stdPresent & XkbOneLevelMask) == 0)
+        i++;
+    if ((info.stdPresent & XkbTwoLevelMask) == 0)
+        i++;
+    if ((info.stdPresent & XkbKeypadMask) == 0)
+        i++;
+    if ((info.stdPresent & XkbAlphabeticMask) == 0)
+        i++;
+
+    if (XkbcAllocClientMap(xkb, XkbKeyTypesMask, i) != Success) {
+        WSGO("Couldn't allocate client map\n");
+        goto err_info;
+    }
 
-            missing = XkbAllRequiredTypes & (~info.stdPresent);
-            keypadVMod = FindKeypadVMod(xkb);
-            if (XkbcInitCanonicalKeyTypes(xkb, missing, keypadVMod) != Success)
-            {
-                FreeKeyTypesInfo(&info);
-                WSGO("Couldn't initialize canonical key types\n");
-                return false;
-            }
-            if (missing & XkbOneLevelMask)
-                xkb->map->types[XkbOneLevelIndex].name =
-                    XkbcAtomGetString(tok_ONE_LEVEL);
-            if (missing & XkbTwoLevelMask)
-                xkb->map->types[XkbTwoLevelIndex].name =
-                    XkbcAtomGetString(tok_TWO_LEVEL);
-            if (missing & XkbAlphabeticMask)
-                xkb->map->types[XkbAlphabeticIndex].name =
-                    XkbcAtomGetString(tok_ALPHABETIC);
-            if (missing & XkbKeypadMask)
-                xkb->map->types[XkbKeypadIndex].name =
-                    XkbcAtomGetString(tok_KEYPAD);
-        }
-        next = &xkb->map->types[XkbLastRequiredType + 1];
-        for (i = 0, def = info.types; i < info.nTypes; i++)
-        {
-            if (def->name == tok_ONE_LEVEL)
-                type = &xkb->map->types[XkbOneLevelIndex];
-            else if (def->name == tok_TWO_LEVEL)
-                type = &xkb->map->types[XkbTwoLevelIndex];
-            else if (def->name == tok_ALPHABETIC)
-                type = &xkb->map->types[XkbAlphabeticIndex];
-            else if (def->name == tok_KEYPAD)
-                type = &xkb->map->types[XkbKeypadIndex];
-            else
-                type = next++;
-            DeleteLevel1MapEntries(def);
-            if (!CopyDefToKeyType(xkb, type, def)) {
-                FreeKeyTypesInfo(&info);
-                return false;
-            }
-            def = (KeyTypeInfo *) def->defs.next;
+    xkb->map->num_types = i;
+
+    if (XkbAllRequiredTypes & (~info.stdPresent)) {
+        unsigned missing, keypadVMod;
+
+        missing = XkbAllRequiredTypes & (~info.stdPresent);
+        keypadVMod = FindKeypadVMod(xkb);
+
+        if (XkbcInitCanonicalKeyTypes(xkb, missing, keypadVMod) != Success) {
+            WSGO("Couldn't initialize canonical key types\n");
+            goto err_info;
         }
-        FreeKeyTypesInfo(&info);
-        return true;
+
+        if (missing & XkbOneLevelMask)
+            xkb->map->types[XkbOneLevelIndex].name =
+                XkbcAtomGetString(tok_ONE_LEVEL);
+        if (missing & XkbTwoLevelMask)
+            xkb->map->types[XkbTwoLevelIndex].name =
+                XkbcAtomGetString(tok_TWO_LEVEL);
+        if (missing & XkbAlphabeticMask)
+            xkb->map->types[XkbAlphabeticIndex].name =
+                XkbcAtomGetString(tok_ALPHABETIC);
+        if (missing & XkbKeypadMask)
+            xkb->map->types[XkbKeypadIndex].name =
+                XkbcAtomGetString(tok_KEYPAD);
     }
 
+    next = &xkb->map->types[XkbLastRequiredType + 1];
+    for (i = 0, def = info.types; i < info.nTypes; i++) {
+        if (def->name == tok_ONE_LEVEL)
+            type = &xkb->map->types[XkbOneLevelIndex];
+        else if (def->name == tok_TWO_LEVEL)
+            type = &xkb->map->types[XkbTwoLevelIndex];
+        else if (def->name == tok_ALPHABETIC)
+            type = &xkb->map->types[XkbAlphabeticIndex];
+        else if (def->name == tok_KEYPAD)
+            type = &xkb->map->types[XkbKeypadIndex];
+        else
+            type = next++;
+
+        DeleteLevel1MapEntries(def);
+
+        if (!CopyDefToKeyType(xkb, type, def))
+            goto err_info;
+
+        def = (KeyTypeInfo *)def->defs.next;
+    }
+
+    FreeKeyTypesInfo(&info);
+    return true;
+
+err_info:
     FreeKeyTypesInfo(&info);
     return false;
 }

diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index 9582d8e..f380edf 100644 (file)
--- a/src/xkbcomp/symbols.c
+++ b/src/xkbcomp/symbols.c
@@ -2178,105 +2178,96 @@ CopyModMapDef(struct xkb_keymap * xkb, ModMapEntry *entry)
  * @param merge Merge strategy (e.g. MergeOverride).
  */
 bool
-CompileSymbols(XkbFile *file, struct xkb_keymap * xkb, unsigned merge)
+CompileSymbols(XkbFile *file, struct xkb_keymap *xkb, unsigned merge)
 {
     unsigned int i;
     SymbolsInfo info;
+    KeyInfo *key;
 
     InitSymbolsInfo(&info, xkb);
     info.dflt.defs.fileID = file->id;
     info.dflt.defs.merge = merge;
+
     HandleSymbolsFile(file, xkb, merge, &info);
 
-    if (info.nKeys == 0) {
-        FreeSymbolsInfo(&info);
-        return false;
+    if (info.nKeys == 0)
+        goto err_info;
+
+    if (info.errorCount != 0)
+        goto err_info;
+
+    /* alloc memory in the xkb struct */
+    if (XkbcAllocNames(xkb, XkbGroupNamesMask, 0) != Success) {
+        WSGO("Can not allocate names in CompileSymbols\n");
+        ACTION("Symbols not added\n");
+        goto err_info;
     }
 
-    if (info.errorCount == 0)
-    {
-        KeyInfo *key;
+    if (XkbcAllocClientMap(xkb, XkbKeySymsMask | XkbModifierMapMask, 0)
+        != Success) {
+        WSGO("Could not allocate client map in CompileSymbols\n");
+        ACTION("Symbols not added\n");
+        goto err_info;
+    }
 
-        /* alloc memory in the xkb struct */
-        if (XkbcAllocNames(xkb, XkbGroupNamesMask, 0) != Success)
-        {
-            WSGO("Can not allocate names in CompileSymbols\n");
-            ACTION("Symbols not added\n");
-            return false;
-        }
-        if (XkbcAllocClientMap(xkb, XkbKeySymsMask | XkbModifierMapMask, 0)
-            != Success)
-        {
-            WSGO("Could not allocate client map in CompileSymbols\n");
-            ACTION("Symbols not added\n");
-            return false;
-        }
-        if (XkbcAllocServerMap(xkb, XkbAllServerInfoMask, 32) != Success)
-        {
-            WSGO("Could not allocate server map in CompileSymbols\n");
-            ACTION("Symbols not added\n");
-            return false;
-        }
-        if (XkbcAllocControls(xkb) != Success)
-        {
-            WSGO("Could not allocate controls in CompileSymbols\n");
-            ACTION("Symbols not added\n");
-            return false;
+    if (XkbcAllocServerMap(xkb, XkbAllServerInfoMask, 32) != Success) {
+        WSGO("Could not allocate server map in CompileSymbols\n");
+        ACTION("Symbols not added\n");
+        goto err_info;
+    }
+
+    if (XkbcAllocControls(xkb) != Success) {
+        WSGO("Could not allocate controls in CompileSymbols\n");
+        ACTION("Symbols not added\n");
+        goto err_info;
+    }
+
+    /* now copy info into xkb. */
+    ApplyAliases(xkb, &info.aliases);
+
+    for (i = 0; i < XkbNumKbdGroups; i++) {
+        if (info.groupNames[i] != XKB_ATOM_NONE) {
+            free(UNCONSTIFY(xkb->names->groups[i]));
+            xkb->names->groups[i] = XkbcAtomGetString(info.groupNames[i]);
         }
+    }
 
-        /* now copy info into xkb. */
-        if (info.aliases)
-            ApplyAliases(xkb, &info.aliases);
-        for (i = 0; i < XkbNumKbdGroups; i++)
-        {
-            if (info.groupNames[i] != XKB_ATOM_NONE)
-            {
-                free(UNCONSTIFY(xkb->names->groups[i]));
-                xkb->names->groups[i] = XkbcAtomGetString(info.groupNames[i]);
+    /* sanitize keys */
+    for (key = info.keys, i = 0; i < info.nKeys; i++, key++)
+        PrepareKeyDef(key);
+
+    /* copy! */
+    for (key = info.keys, i = 0; i < info.nKeys; i++, key++)
+        if (!CopySymbolsDef(xkb, key, 0))
+            info.errorCount++;
+
+    if (warningLevel > 3) {
+        for (i = xkb->min_key_code; i <= xkb->max_key_code; i++) {
+            if (xkb->names->keys[i].name[0] == '\0')
+                continue;
+
+            if (XkbKeyNumGroups(xkb, i) < 1) {
+                char buf[5];
+                memcpy(buf, xkb->names->keys[i].name, 4);
+                buf[4] = '\0';
+                WARN("No symbols defined for <%s> (keycode %d)\n", buf, i);
             }
         }
-        /* sanitize keys */
-        for (key = info.keys, i = 0; i < info.nKeys; i++, key++)
-        {
-            PrepareKeyDef(key);
-        }
-        /* copy! */
-        for (key = info.keys, i = 0; i < info.nKeys; i++, key++)
-        {
-            if (!CopySymbolsDef(xkb, key, 0))
+    }
+
+    if (info.modMap) {
+        ModMapEntry *mm, *next;
+        for (mm = info.modMap; mm != NULL; mm = next) {
+            if (!CopyModMapDef(xkb, mm))
                 info.errorCount++;
+            next = (ModMapEntry *)mm->defs.next;
         }
-        if (warningLevel > 3)
-        {
-            for (i = xkb->min_key_code; i <= xkb->max_key_code; i++)
-            {
-                if (xkb->names->keys[i].name[0] == '\0')
-                    continue;
-                if (XkbKeyNumGroups(xkb, i) < 1)
-                {
-                    char buf[5];
-                    memcpy(buf, xkb->names->keys[i].name, 4);
-                    buf[4] = '\0';
-                    WARN
-                        ("No symbols defined for <%s> (keycode %d)\n",
-                         buf, i);
-                }
-            }
-        }
-        if (info.modMap)
-        {
-            ModMapEntry *mm, *next;
-            for (mm = info.modMap; mm != NULL; mm = next)
-            {
-                if (!CopyModMapDef(xkb, mm))
-                    info.errorCount++;
-                next = (ModMapEntry *) mm->defs.next;
-            }
-        }
-        FreeSymbolsInfo(&info);
-        return true;
     }
 
     FreeSymbolsInfo(&info);
+    return true;
+
+err_info:
+    FreeSymbolsInfo(&info);
     return false;
 }