Some atom related optimizations

We often get a strdup'd string, just to pass it over the atom_intern and
then immediately free it. But atom_intern then strdup's it again (if
it's not interned already); so instead we can have the interning "steal"
the memory instead of allocing a new one and freeing the old one. This
is done by a new xkb_atom_steal function.

It also turns out, that every time we strdup an atom, we don't actually
modify it afterwards. Since we are guaranteed that the atom table will
live as long as the context, we can just use xkb_atom_text instead. This
removes a some more dynamic allocations.

For this change we had to remove the ability to append two strings, e.g.
    "foo" + "bar" -> "foobar"
which is only possible with string literals. This is unused and quite
useless for our purposes.

xkb_atom_strdup is left unused, as it may still be useful.

Running rulescomp in valgrind, Before:
==7907== total heap usage: 173,698 allocs, 173,698 frees, 9,775,973 bytes allocated
After:
==6348== total heap usage: 168,403 allocs, 168,403 frees, 9,732,648 bytes allocated

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

src/alloc.c

@@ -36,8 +36,6 @@ XkbcCopyKeyType(const struct xkb_key_type *from, struct xkb_key_type *into)
 
     darray_free(into->map);
     free(into->preserve);
-    for (i = 0; i < into->num_levels; i++)
-        free(into->level_names[i]);
     free(into->level_names);
 
     *into = *from;
@@ -113,13 +111,9 @@ free_types(struct xkb_keymap *keymap)
     struct xkb_key_type *type;
 
     darray_foreach(type, keymap->types) {
-        int j;
         darray_free(type->map);
         free(type->preserve);
-        for (j = 0; j < type->num_levels; j++)
-            free(type->level_names[j]);
         free(type->level_names);
-        free(type->name);
     }
     darray_free(keymap->types);
 }
@@ -138,28 +132,6 @@ free_keys(struct xkb_keymap *keymap)
     darray_free(keymap->keys);
 }
 
-static void
-free_names(struct xkb_keymap *keymap)
-{
-    int i;
-
-    for (i = 0; i < XkbNumVirtualMods; i++)
-        free(keymap->vmod_names[i]);
-
-    for (i = 0; i < XkbNumIndicators; i++)
-        free(keymap->indicator_names[i]);
-
-    for (i = 0; i < XkbNumKbdGroups; i++)
-        free(keymap->group_names[i]);
-
-    darray_free(keymap->key_aliases);
-
-    free(keymap->keycodes_section_name);
-    free(keymap->symbols_section_name);
-    free(keymap->types_section_name);
-    free(keymap->compat_section_name);
-}
-
 struct xkb_keymap *
 XkbcAllocKeyboard(struct xkb_context *ctx)
 {
@@ -184,7 +156,11 @@ XkbcFreeKeyboard(struct xkb_keymap *keymap)
     free_types(keymap);
     darray_free(keymap->acts);
     darray_free(keymap->sym_interpret);
-    free_names(keymap);
+    darray_free(keymap->key_aliases);
+    free(keymap->keycodes_section_name);
+    free(keymap->symbols_section_name);
+    free(keymap->types_section_name);
+    free(keymap->compat_section_name);
     free_keys(keymap);
     xkb_context_unref(keymap->ctx);
     free(keymap);

src/atom.c

@@ -140,8 +140,14 @@ atom_strdup(struct atom_table *table, xkb_atom_t atom)
     return ret ? strdup(ret) : NULL;
 }
 
+/*
+ * If steal is true, we do not strdup @string; therefore it must be
+ * dynamically allocated, not be free'd by the caller and not be used
+ * afterwards. Use to avoid some redundant allocations.
+ */
 xkb_atom_t
-atom_intern(struct atom_table *table, const char *string)
+atom_intern(struct atom_table *table, const char *string,
+            bool steal)
 {
     struct atom_node **np;
     struct atom_node *nd;
@@ -168,26 +174,34 @@ atom_intern(struct atom_table *table, const char *string)
         else {
             /* now start testing the strings */
             comp = strncmp(string, (*np)->string, len);
-            if ((comp < 0) || ((comp == 0) && (len < strlen((*np)->string))))
+            if (comp < 0 || (comp == 0 && len < strlen((*np)->string))) {
                 np = &((*np)->left);
-            else if (comp > 0)
+            }
+            else if (comp > 0) {
                 np = &((*np)->right);
-            else
+            }
+            else {
+                if (steal)
+                    free(UNCONSTIFY(string));
                 return (*np)->a;
             }
         }
+    }
 
     nd = malloc(sizeof(*nd));
     if (!nd)
         return XKB_ATOM_NONE;
 
-    nd->string = malloc(len + 1);
+    if (steal) {
+        nd->string = UNCONSTIFY(string);
+    }
+    else {
+        nd->string = strdup(string);
         if (!nd->string) {
             free(nd);
             return XKB_ATOM_NONE;
         }
-    strncpy(nd->string, string, len);
+    }
-    nd->string[len] = 0;
 
     *np = nd;
     nd->left = nd->right = NULL;

src/atom.h

@@ -38,7 +38,8 @@ void
 atom_table_free(struct atom_table *table);
 
 xkb_atom_t
-atom_intern(struct atom_table *table, const char *string);
+atom_intern(struct atom_table *table, const char *string,
+            bool steal);
 
 char *
 atom_strdup(struct atom_table *table, xkb_atom_t atom);

src/context.c

@@ -295,7 +295,13 @@ xkb_context_new(enum xkb_context_flags flags)
 xkb_atom_t
 xkb_atom_intern(struct xkb_context *ctx, const char *string)
 {
-    return atom_intern(ctx->atom_table, string);
+    return atom_intern(ctx->atom_table, string, false);
+}
+
+xkb_atom_t
+xkb_atom_steal(struct xkb_context *ctx, char *string)
+{
+    return atom_intern(ctx->atom_table, string, true);
 }
 
 char *

src/xkb-priv.h

@@ -250,8 +250,8 @@ struct xkb_key_type {
     uint16_t num_levels;
     darray(struct xkb_kt_map_entry) map;
     struct xkb_mods *preserve;
-    char *name;
+    const char *name;
-    char **level_names;
+    const char **level_names;
 };
 
 struct xkb_sym_interpret {
@@ -352,15 +352,15 @@ struct xkb_keymap {
 
     /* vmod -> mod mapping */
     uint32_t vmods[XkbNumVirtualMods];
-    char *vmod_names[XkbNumVirtualMods];
+    const char *vmod_names[XkbNumVirtualMods];
 
     struct xkb_mods groups[XkbNumKbdGroups];
-    char *group_names[XkbNumKbdGroups];
+    const char *group_names[XkbNumKbdGroups];
 
     darray(union xkb_action) acts;
 
     struct xkb_indicator_map indicators[XkbNumIndicators];
-    char *indicator_names[XkbNumIndicators];
+    const char *indicator_names[XkbNumIndicators];
 
     char *keycodes_section_name;
     char *symbols_section_name;
@@ -474,6 +474,15 @@ typedef uint32_t xkb_atom_t;
 xkb_atom_t
 xkb_atom_intern(struct xkb_context *ctx, const char *string);
 
+/**
+ * If @string is dynamically allocated, free'd immediately after
+ * being interned, and not used afterwards, use this function
+ * instead of xkb_atom_intern to avoid some unnecessary allocations.
+ * The caller should not use or free the passed in string afterwards.
+ */
+xkb_atom_t
+xkb_atom_steal(struct xkb_context *ctx, char *string);
+
 char *
 xkb_atom_strdup(struct xkb_context *ctx, xkb_atom_t atom);
 

src/xkbcomp/action.c

@@ -980,7 +980,6 @@ HandlePrivate(struct xkb_keymap *keymap, struct xkb_any_action *action,
                 }
                 strncpy((char *) action->data, rtrn.str, sizeof action->data);
             }
-            free(rtrn.str);
             return true;
         }
         else {
@@ -1133,18 +1132,12 @@ HandleActionDef(ExprDef * def,
                     "Cannot change defaults in an action definition; "
                     "Ignoring attempt to change %s.%s\n",
                     elemRtrn.str, fieldRtrn.str);
-            free(elemRtrn.str);
-            free(fieldRtrn.str);
             return false;
         }
         if (!stringToField(fieldRtrn.str, &fieldNdx)) {
             log_err(keymap->ctx, "Unknown field name %s\n", fieldRtrn.str);
-            free(elemRtrn.str);
-            free(fieldRtrn.str);
             return false;
         }
-        free(elemRtrn.str);
-        free(fieldRtrn.str);
         if (!(*handleAction[hndlrType])(keymap, action, fieldNdx, arrayRtrn,
                                         value))
             return false;
@@ -1155,7 +1148,7 @@ HandleActionDef(ExprDef * def,
 /***====================================================================***/
 
 int
-SetActionField(struct xkb_keymap *keymap, char *elem, char *field,
+SetActionField(struct xkb_keymap *keymap, const char *elem, const char *field,
                ExprDef *array_ndx, ExprDef *value, ActionInfo **info_rtrn)
 {
     ActionInfo *new, *old;

src/xkbcomp/action.h

@@ -72,7 +72,7 @@ HandleActionDef(ExprDef *def, struct xkb_keymap *keymap,
                 ActionInfo *info);
 
 extern int
-SetActionField(struct xkb_keymap *keymap, char *elem, char *field,
+SetActionField(struct xkb_keymap *keymap, const char *elem, const char *field,
                ExprDef *index, ExprDef *value,
                ActionInfo **info_rtrn);
 

src/xkbcomp/compat.c

@@ -637,7 +637,7 @@ static const LookupEntry useModMapValues[] = {
 };
 
 static int
-SetInterpField(CompatInfo *info, SymInterpInfo *si, char *field,
+SetInterpField(CompatInfo *info, SymInterpInfo *si, const char *field,
                ExprDef *arrayNdx, ExprDef *value)
 {
     int ok = 1;
@@ -750,7 +750,7 @@ static const LookupEntry groupNames[] = {
 
 static int
 SetIndicatorMapField(CompatInfo *info, LEDInfo *led,
-                     char *field, ExprDef *arrayNdx, ExprDef *value)
+                     const char *field, ExprDef *arrayNdx, ExprDef *value)
 {
     ExprResult rtrn;
     bool ok = true;
@@ -889,8 +889,6 @@ HandleInterpVar(CompatInfo *info, VarDef *stmt)
     else
         ret = SetActionField(info->keymap, elem.str, field.str, ndx,
                              stmt->value, &info->act);
-    free(elem.str);
-    free(field.str);
     return ret;
 }
 
@@ -909,7 +907,6 @@ HandleInterpBody(CompatInfo *info, VarDef *def, SymInterpInfo *si)
         ok = ExprResolveLhs(info->keymap, def->name, &tmp, &field, &arrayNdx);
         if (ok) {
             ok = SetInterpField(info, si, field.str, arrayNdx, def->value);
-            free(field.str);
         }
     }
     return ok;
@@ -1020,8 +1017,6 @@ HandleIndicatorMapDef(CompatInfo *info, IndicatorMapDef *def,
             ok = SetIndicatorMapField(info, &led, field.str, arrayNdx,
                                       var->value) && ok;
         }
-        free(elem.str);
-        free(field.str);
     }
 
     if (ok)
@@ -1130,7 +1125,7 @@ BindIndicators(CompatInfo *info, struct list *unbound_leds)
             for (i = 0; i < XkbNumIndicators; i++) {
                 if (keymap->indicator_names[i] == NULL) {
                     keymap->indicator_names[i] =
-                        xkb_atom_strdup(keymap->ctx, led->name);
+                        xkb_atom_text(keymap->ctx, led->name);
                     led->indicator = i + 1;
                     break;
                 }
@@ -1210,9 +1205,8 @@ CopyIndicatorMapDefs(CompatInfo *info)
         im->mods.real_mods = led->real_mods;
         im->mods.vmods = led->vmods;
         im->ctrls = led->ctrls;
-        free(keymap->indicator_names[led->indicator - 1]);
         keymap->indicator_names[led->indicator - 1] =
-            xkb_atom_strdup(keymap->ctx, led->name);
+            xkb_atom_text(keymap->ctx, led->name);
         free(led);
     }
     list_init(&info->leds);

src/xkbcomp/expr.c

@@ -82,7 +82,7 @@ exprOpText(unsigned type)
         strcpy(buf, "bitwise inversion");
         break;
     case OpUnaryPlus:
-        strcpy(buf, "plus sign");
+        strcpy(buf, "unary plus");
         break;
     default:
         snprintf(buf, sizeof(buf), "illegal(%d)", type);
@@ -127,25 +127,22 @@ ExprResolveLhs(struct xkb_keymap *keymap, ExprDef *expr,
                ExprResult *elem_rtrn, ExprResult *field_rtrn,
                ExprDef **index_rtrn)
 {
+    struct xkb_context *ctx = keymap->ctx;
+
     switch (expr->op) {
     case ExprIdent:
         elem_rtrn->str = NULL;
-        field_rtrn->str = xkb_atom_strdup(keymap->ctx,
+        field_rtrn->str = xkb_atom_text(ctx, expr->value.str);
-                                          expr->value.str);
         *index_rtrn = NULL;
         return true;
     case ExprFieldRef:
-        elem_rtrn->str = xkb_atom_strdup(keymap->ctx,
+        elem_rtrn->str = xkb_atom_text(ctx, expr->value.field.element);
-                                         expr->value.field.element);
+        field_rtrn->str = xkb_atom_text(ctx, expr->value.field.field);
-        field_rtrn->str = xkb_atom_strdup(keymap->ctx,
-                                          expr->value.field.field);
         *index_rtrn = NULL;
         return true;
     case ExprArrayRef:
-        elem_rtrn->str = xkb_atom_strdup(keymap->ctx,
+        elem_rtrn->str = xkb_atom_text(ctx, expr->value.array.element);
-                                         expr->value.array.element);
+        field_rtrn->str = xkb_atom_text(ctx, expr->value.array.field);
-        field_rtrn->str = xkb_atom_strdup(keymap->ctx,
-                                          expr->value.array.field);
         *index_rtrn = expr->value.array.entry;
         return true;
     }
@@ -268,29 +265,14 @@ ExprResolveBoolean(struct xkb_context *ctx, ExprDef *expr,
             val_rtrn->uval = !val_rtrn->uval;
         return ok;
     case OpAdd:
-        if (bogus == NULL)
-            bogus = "Addition";
     case OpSubtract:
-        if (bogus == NULL)
-            bogus = "Subtraction";
     case OpMultiply:
-        if (bogus == NULL)
-            bogus = "Multiplication";
     case OpDivide:
-        if (bogus == NULL)
-            bogus = "Division";
     case OpAssign:
-        if (bogus == NULL)
-            bogus = "Assignment";
     case OpNegate:
-        if (bogus == NULL)
-            bogus = "Negation";
-        log_err(ctx, "%s of boolean values not permitted\n", bogus);
-        break;
-
     case OpUnaryPlus:
-        log_err(ctx,
+        log_err(ctx, "%s of boolean values not permitted\n",
-                "Unary \"+\" operator not permitted for boolean values\n");
+                exprOpText(expr->op));
         break;
 
     default:
@@ -589,11 +571,6 @@ int
 ExprResolveString(struct xkb_context *ctx, ExprDef *expr,
                   ExprResult *val_rtrn)
 {
-    ExprResult leftRtrn, rightRtrn;
-    ExprDef *left;
-    ExprDef *right;
-    const char *bogus = NULL;
-
     switch (expr->op) {
     case ExprValue:
         if (expr->type != TypeString) {
@@ -601,9 +578,7 @@ ExprResolveString(struct xkb_context *ctx, ExprDef *expr,
                     exprTypeText(expr->type));
             return false;
         }
-        val_rtrn->str = xkb_atom_strdup(ctx, expr->value.str);
+        val_rtrn->str = xkb_atom_text(ctx, expr->value.str);
-        if (val_rtrn->str == NULL)
-            val_rtrn->str = strdup("");
         return true;
 
     case ExprIdent:
@@ -618,53 +593,15 @@ ExprResolveString(struct xkb_context *ctx, ExprDef *expr,
         return false;
 
     case OpAdd:
-        left = expr->value.binary.left;
-        right = expr->value.binary.right;
-        if (ExprResolveString(ctx, left, &leftRtrn) &&
-            ExprResolveString(ctx, right, &rightRtrn)) {
-            int len;
-            char *new;
-            len = strlen(leftRtrn.str) + strlen(rightRtrn.str) + 1;
-            new = malloc(len);
-            if (new) {
-                sprintf(new, "%s%s", leftRtrn.str, rightRtrn.str);
-                free(leftRtrn.str);
-                free(rightRtrn.str);
-                val_rtrn->str = new;
-                return true;
-            }
-            free(leftRtrn.str);
-            free(rightRtrn.str);
-        }
-        return false;
-
     case OpSubtract:
-        if (bogus == NULL)
-            bogus = "Subtraction";
     case OpMultiply:
-        if (bogus == NULL)
-            bogus = "Multiplication";
     case OpDivide:
-        if (bogus == NULL)
-            bogus = "Division";
     case OpAssign:
-        if (bogus == NULL)
-            bogus = "Assignment";
     case OpNegate:
-        if (bogus == NULL)
-            bogus = "Negation";
     case OpInvert:
-        if (bogus == NULL)
-            bogus = "Bitwise complement";
-        log_err(ctx, "%s of string values not permitted\n", bogus);
-        return false;
-
     case OpNot:
-        log_err(ctx, "The ! operator cannot be applied to a string\n");
-        return false;
-
     case OpUnaryPlus:
-        log_err(ctx, "The + operator cannot be applied to a string\n");
+        log_err(ctx, "%s of strings not permitted\n", exprOpText(expr->op));
         return false;
 
     default:
@@ -678,8 +615,6 @@ int
 ExprResolveKeyName(struct xkb_context *ctx, ExprDef *expr,
                    ExprResult *val_rtrn)
 {
-    const char *bogus = NULL;
-
     switch (expr->op) {
     case ExprValue:
         if (expr->type != TypeKeyName) {
@@ -702,35 +637,16 @@ ExprResolveKeyName(struct xkb_context *ctx, ExprDef *expr,
         return false;
 
     case OpAdd:
-        if (bogus == NULL)
-            bogus = "Addition";
     case OpSubtract:
-        if (bogus == NULL)
-            bogus = "Subtraction";
     case OpMultiply:
-        if (bogus == NULL)
-            bogus = "Multiplication";
     case OpDivide:
-        if (bogus == NULL)
-            bogus = "Division";
     case OpAssign:
-        if (bogus == NULL)
-            bogus = "Assignment";
     case OpNegate:
-        if (bogus == NULL)
-            bogus = "Negation";
     case OpInvert:
-        if (bogus == NULL)
-            bogus = "Bitwise complement";
-        log_err(ctx, "%s of key name values not permitted\n", bogus);
-        return false;
-
     case OpNot:
-        log_err(ctx, "The ! operator cannot be applied to a key name\n");
-        return false;
-
     case OpUnaryPlus:
-        log_err(ctx, "The + operator cannot be applied to a key name\n");
+        log_err(ctx, "%s of key name values not permitted\n",
+                exprOpText(expr->op));
         return false;
 
     default:

src/xkbcomp/expr.h

@@ -30,7 +30,7 @@
 #include "xkbcomp-priv.h"
 
 typedef union _ExprResult {
-    char *str;
+    const char *str;
     int ival;
     unsigned uval;
     char name[XkbKeyNameLength];

src/xkbcomp/keycodes.c

@@ -692,11 +692,9 @@ HandleKeyNameVar(KeyNamesInfo *info, VarDef *stmt)
         info->explicitMax = tmp.uval;
     }
 
-    free(field.str);
     return 1;
 
 err_out:
-    free(field.str);
     return 0;
 }
 
@@ -724,11 +722,9 @@ HandleIndicatorNameDef(KeyNamesInfo *info, IndicatorNameDef *def,
                              "string");
     }
     ii.name = xkb_atom_intern(info->keymap->ctx, tmp.str);
-    free(tmp.str);
     ii.virtual = def->virtual;
-    if (!AddIndicatorName(info, merge, &ii))
+
-        return false;
+    return AddIndicatorName(info, merge, &ii);
-    return true;
 }
 
 /**
@@ -927,9 +923,8 @@ CompileKeycodes(XkbFile *file, struct xkb_keymap *keymap,
         keymap->keycodes_section_name = strdup(info.name);
 
     list_foreach(ii, &info.leds, entry) {
-        free(keymap->indicator_names[ii->ndx - 1]);
         keymap->indicator_names[ii->ndx - 1] =
-            xkb_atom_strdup(keymap->ctx, ii->name);
+            xkb_atom_text(keymap->ctx, ii->name);
     }
 
     ApplyAliases(&info);

src/xkbcomp/keytypes.c

@@ -685,7 +685,6 @@ SetLevelName(KeyTypesInfo *info, KeyTypeInfo *type, ExprDef *arrayNdx,
         return false;
     }
     level_name = xkb_atom_intern(ctx, rtrn.str);
-    free(rtrn.str);
     return AddLevelName(info, type, level, level_name, true);
 }
 
@@ -698,7 +697,7 @@ SetLevelName(KeyTypesInfo *info, KeyTypeInfo *type, ExprDef *arrayNdx,
  */
 static bool
 SetKeyTypeField(KeyTypesInfo *info, KeyTypeInfo *type,
-                char *field, ExprDef *arrayNdx, ExprDef *value)
+                const char *field, ExprDef *arrayNdx, ExprDef *value)
 {
     ExprResult tmp;
 
@@ -792,7 +791,6 @@ HandleKeyTypeBody(KeyTypesInfo *info, VarDef *def, KeyTypeInfo *type)
         if (ok) {
             ok = SetKeyTypeField(info, type, field.str, arrayNdx,
                                  def->value);
-            free(field.str);
         }
     }
     return ok;
@@ -1001,7 +999,7 @@ CopyDefToKeyType(KeyTypesInfo *info, KeyTypeInfo *def,
     }
     else
         type->preserve = NULL;
-    type->name = xkb_atom_strdup(keymap->ctx, def->name);
+    type->name = xkb_atom_text(keymap->ctx, def->name);
 
     if (!darray_empty(def->lvlNames)) {
         type->level_names = calloc(darray_size(def->lvlNames),
@@ -1010,7 +1008,7 @@ CopyDefToKeyType(KeyTypesInfo *info, KeyTypeInfo *def,
         /* assert def->szNames<=def->numLevels */
         for (i = 0; i < darray_size(def->lvlNames); i++)
             type->level_names[i] =
-                xkb_atom_strdup(keymap->ctx, darray_item(def->lvlNames, i));
+                xkb_atom_text(keymap->ctx, darray_item(def->lvlNames, i));
     }
     else {
         type->level_names = NULL;
@@ -1193,16 +1191,16 @@ CompileKeyTypes(XkbFile *file, struct xkb_keymap *keymap,
 
         if (missing & XkbOneLevelMask)
             darray_item(keymap->types, XkbOneLevelIndex).name =
-                xkb_atom_strdup(keymap->ctx, info.tok_ONE_LEVEL);
+                xkb_atom_text(keymap->ctx, info.tok_ONE_LEVEL);
         if (missing & XkbTwoLevelMask)
             darray_item(keymap->types, XkbTwoLevelIndex).name =
-                xkb_atom_strdup(keymap->ctx, info.tok_TWO_LEVEL);
+                xkb_atom_text(keymap->ctx, info.tok_TWO_LEVEL);
         if (missing & XkbAlphabeticMask)
             darray_item(keymap->types, XkbAlphabeticIndex).name =
-                xkb_atom_strdup(keymap->ctx, info.tok_ALPHABETIC);
+                xkb_atom_text(keymap->ctx, info.tok_ALPHABETIC);
         if (missing & XkbKeypadMask)
             darray_item(keymap->types, XkbKeypadIndex).name =
-                xkb_atom_strdup(keymap->ctx, info.tok_KEYPAD);
+                xkb_atom_text(keymap->ctx, info.tok_KEYPAD);
     }
 
     next = &darray_item(keymap->types, XkbLastRequiredType + 1);

src/xkbcomp/parser.y

@@ -780,8 +780,7 @@ KeyName		:	KEYNAME		{ $$= $1; }
 
 Ident		:	IDENT
                         {
-                            $$ = xkb_atom_intern(param->ctx, $1);
+                            $$ = xkb_atom_steal(param->ctx, $1);
-                            free($1);
                         }
 		|	DEFAULT
                         {
@@ -791,8 +790,7 @@ Ident		:	IDENT
 
 String		:	STRING
                         {
-                            $$ = xkb_atom_intern(param->ctx, $1);
+                            $$ = xkb_atom_steal(param->ctx, $1);
-                            free($1);
                         }
 		;
 

src/xkbcomp/symbols.c

@@ -1021,7 +1021,7 @@ static const LookupEntry repeatEntries[] = {
 };
 
 static bool
-SetSymbolsField(SymbolsInfo *info, KeyInfo *keyi, char *field,
+SetSymbolsField(SymbolsInfo *info, KeyInfo *keyi, const char *field,
                 ExprDef *arrayNdx, ExprDef *value)
 {
     bool ok = true;
@@ -1043,14 +1043,12 @@ SetSymbolsField(SymbolsInfo *info, KeyInfo *keyi, char *field,
                     "Illegal group index for type of key %s; "
                     "Definition with non-integer array index ignored\n",
                     longText(keyi->name));
-            free(tmp.str);
             return false;
         }
         else {
             keyi->types[ndx.uval - 1] = xkb_atom_intern(ctx, tmp.str);
             keyi->typesDefined |= (1 << (ndx.uval - 1));
         }
-        free(tmp.str);
     }
     else if (strcasecmp(field, "symbols") == 0)
         return AddSymbolsToKey(info, keyi, arrayNdx, value);
@@ -1192,7 +1190,6 @@ SetGroupName(SymbolsInfo *info, ExprDef *arrayNdx, ExprDef *value)
 
     info->groupNames[tmp.uval - 1 + info->explicit_group] =
         xkb_atom_intern(info->keymap->ctx, name.str);
-    free(name.str);
 
     return true;
 }
@@ -1249,8 +1246,6 @@ HandleSymbolsVar(SymbolsInfo *info, VarDef *stmt)
                              stmt->value, &info->action);
     }
 
-    free(elem.str);
-    free(field.str);
     return ret;
 }
 
@@ -1266,25 +1261,24 @@ HandleSymbolsBody(SymbolsInfo *info, VarDef *def, KeyInfo *keyi)
             ok = HandleSymbolsVar(info, def);
             continue;
         }
-        else {
+
-            if (def->name == NULL) {
+        if (!def->name) {
-                if ((def->value == NULL)
+            if (!def->value || (def->value->op == ExprKeysymList))
-                    || (def->value->op == ExprKeysymList))
+                field.str = "symbols";
-                    field.str = strdup("symbols");
             else
-                    field.str = strdup("actions");
+                field.str = "actions";
             arrayNdx = NULL;
         }
         else {
             ok = ExprResolveLhs(info->keymap, def->name, &tmp, &field,
                                 &arrayNdx);
         }
+
         if (ok)
             ok = SetSymbolsField(info, keyi, field.str, arrayNdx,
                                  def->value);
-            free(field.str);
-        }
     }
+
     return ok;
 }
 
@@ -1949,8 +1943,7 @@ CompileSymbols(XkbFile *file, struct xkb_keymap *keymap,
 
     for (i = 0; i < XkbNumKbdGroups; i++) {
         if (info.groupNames[i] != XKB_ATOM_NONE) {
-            free(keymap->group_names[i]);
+            keymap->group_names[i] = xkb_atom_text(keymap->ctx,
-            keymap->group_names[i] = xkb_atom_strdup(keymap->ctx,
                                                    info.groupNames[i]);
         }
     }

src/xkbcomp/vmod.c

@@ -120,7 +120,7 @@ HandleVModDef(VModDef *stmt, struct xkb_keymap *keymap,
     info->defined |= (1 << nextFree);
     info->newlyDefined |= (1 << nextFree);
     info->available |= (1 << nextFree);
-    keymap->vmod_names[nextFree] = xkb_atom_strdup(keymap->ctx, stmt->name);
+    keymap->vmod_names[nextFree] = xkb_atom_text(keymap->ctx, stmt->name);
     if (stmt->value == NULL)
         return true;
     if (ExprResolveModMask(keymap->ctx, stmt->value, &mod)) {