From 670c76bdadd34ffe0b50d47fe126a1f5aee41af6 Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Tue, 21 Nov 2023 08:46:06 +0100 Subject: [PATCH 1/3] utils: Steal `steal` from libei Add excerpt of `util-mem.h` from libei defining the macro `steal`, in order to improve memory management and the code semantics. See: https://gitlab.freedesktop.org/libinput/libei/-/blob/38132d6fc5905e2d4361325fdded29a0dff990d6/src/util-mem.h#L92 --- src/util-mem.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/util-mem.h diff --git a/src/util-mem.h b/src/util-mem.h new file mode 100644 index 000000000..a2e781039 --- /dev/null +++ b/src/util-mem.h @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2020 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#pragma once + +#include "config.h" +#include + +static inline void* +_steal(void *ptr) { + void **original = (void**)ptr; + void *swapped = *original; + *original = NULL; + return swapped; +} + +/** + * Resets the pointer content and resets the data to NULL. + */ +#ifdef _WIN32 +#define steal(ptr_) \ + _steal(ptr_) +#else +#define steal(ptr_) \ + (__typeof__(*ptr_))_steal(ptr_) +#endif From ba85e2579307def3c07f5171edc08d72911cd6d3 Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Tue, 21 Nov 2023 08:50:38 +0100 Subject: [PATCH 2/3] registry: Use `steal` for better memory handling --- meson.build | 1 + src/registry.c | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/meson.build b/meson.build index d5c4c74e4..ded142f72 100644 --- a/meson.build +++ b/meson.build @@ -364,6 +364,7 @@ if get_option('enable-xkbregistry') 'src/utils.c', 'src/util-list.h', 'src/util-list.c', + 'src/util-mem.h', ] libxkbregistry_link_args = [] libxkbregistry_link_deps = [] diff --git a/src/registry.c b/src/registry.c index cd1560e7e..19b2b36c4 100644 --- a/src/registry.c +++ b/src/registry.c @@ -34,6 +34,7 @@ #include "xkbcommon/xkbregistry.h" #include "utils.h" #include "util-list.h" +#include "util-mem.h" struct rxkb_object; @@ -803,9 +804,9 @@ parse_model(struct rxkb_context *ctx, xmlNode *model, /* new model */ m = rxkb_model_create(&ctx->base); - m->name = config.name; - m->description = config.description; - m->vendor = config.vendor; + m->name = steal(&config.name); + m->description = steal(&config.description); + m->vendor = steal(&config.vendor); m->popularity = config.popularity; list_append(&ctx->models, &m->base.link); } @@ -894,10 +895,10 @@ parse_variant(struct rxkb_context *ctx, struct rxkb_layout *l, list_init(&v->iso639s); list_init(&v->iso3166s); v->name = strdup(l->name); - v->variant = config.name; - v->description = config.description; + v->variant = steal(&config.name); + v->description = steal(&config.description); // if variant omits brief, inherit from parent layout. - v->brief = config.brief == NULL ? strdup_safe(l->brief) : config.brief; + v->brief = config.brief == NULL ? strdup_safe(l->brief) : steal(&config.brief); v->popularity = config.popularity; list_append(&ctx->layouts, &v->base.link); @@ -979,10 +980,10 @@ parse_layout(struct rxkb_context *ctx, xmlNode *layout, l = rxkb_layout_create(&ctx->base); list_init(&l->iso639s); list_init(&l->iso3166s); - l->name = config.name; + l->name = steal(&config.name); l->variant = NULL; - l->description = config.description; - l->brief = config.brief; + l->description = steal(&config.description); + l->brief = steal(&config.brief); l->popularity = config.popularity; list_append(&ctx->layouts, &l->base.link); } else { @@ -1034,8 +1035,8 @@ parse_option(struct rxkb_context *ctx, struct rxkb_option_group *group, } o = rxkb_option_create(&group->base); - o->name = config.name; - o->description = config.description; + o->name = steal(&config.name); + o->description = steal(&config.description); o->popularity = config.popularity; list_append(&group->options, &o->base.link); } @@ -1063,8 +1064,8 @@ parse_group(struct rxkb_context *ctx, xmlNode *group, if (!exists) { g = rxkb_option_group_create(&ctx->base); - g->name = config.name; - g->description = config.description; + g->name = steal(&config.name); + g->description = steal(&config.description); g->popularity = config.popularity; multiple = xmlGetProp(group, (const xmlChar*)"allowMultipleSelection"); From 90dedc657c452ddfb25fdfeeddaa7e8e81c99fdd Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Thu, 23 Nov 2023 09:30:57 +0100 Subject: [PATCH 3/3] xkbcomp: Use `steal` for better memory handling --- meson.build | 1 + src/xkbcomp/compat.c | 7 +++---- src/xkbcomp/keycodes.c | 7 +++---- src/xkbcomp/symbols.c | 7 +++---- src/xkbcomp/types.c | 7 +++---- 5 files changed, 13 insertions(+), 16 deletions(-) diff --git a/meson.build b/meson.build index ded142f72..25b9ef6da 100644 --- a/meson.build +++ b/meson.build @@ -228,6 +228,7 @@ libxkbcommon_sources = [ 'src/text.h', 'src/utf8.c', 'src/utf8.h', + 'src/util-mem.h', 'src/utils.c', 'src/utils.h', ] diff --git a/src/xkbcomp/compat.c b/src/xkbcomp/compat.c index 1df9e0376..bb45c692e 100644 --- a/src/xkbcomp/compat.c +++ b/src/xkbcomp/compat.c @@ -55,6 +55,7 @@ #include "action.h" #include "vmod.h" #include "include.h" +#include "util-mem.h" enum si_field { SI_FIELD_VIRTUAL_MOD = (1 << 0), @@ -393,8 +394,7 @@ MergeIncludedCompatMaps(CompatInfo *into, CompatInfo *from, into->mods = from->mods; if (into->name == NULL) { - into->name = from->name; - from->name = NULL; + into->name = steal(&from->name); } if (darray_empty(into->interps)) { @@ -440,8 +440,7 @@ HandleIncludeCompatMap(CompatInfo *info, IncludeStmt *include) InitCompatInfo(&included, info->ctx, 0 /* unused */, info->actions, &info->mods); - included.name = include->stmt; - include->stmt = NULL; + included.name = steal(&include->stmt); for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) { CompatInfo next_incl; diff --git a/src/xkbcomp/keycodes.c b/src/xkbcomp/keycodes.c index 700cd1c3f..3e3254d19 100644 --- a/src/xkbcomp/keycodes.c +++ b/src/xkbcomp/keycodes.c @@ -30,6 +30,7 @@ #include "text.h" #include "expr.h" #include "include.h" +#include "util-mem.h" typedef struct { enum merge_mode merge; @@ -268,8 +269,7 @@ MergeIncludedKeycodes(KeyNamesInfo *into, KeyNamesInfo *from, } if (into->name == NULL) { - into->name = from->name; - from->name = NULL; + into->name = steal(&from->name); } /* Merge key names. */ @@ -348,8 +348,7 @@ HandleIncludeKeycodes(KeyNamesInfo *info, IncludeStmt *include) } InitKeyNamesInfo(&included, info->ctx, 0 /* unused */); - included.name = include->stmt; - include->stmt = NULL; + included.name = steal(&include->stmt); for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) { KeyNamesInfo next_incl; diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c index 0c4fbebd6..4492a737e 100644 --- a/src/xkbcomp/symbols.c +++ b/src/xkbcomp/symbols.c @@ -60,6 +60,7 @@ #include "vmod.h" #include "include.h" #include "keysym.h" +#include "util-mem.h" enum key_repeat { @@ -518,8 +519,7 @@ MergeIncludedSymbols(SymbolsInfo *into, SymbolsInfo *from, into->mods = from->mods; if (into->name == NULL) { - into->name = from->name; - from->name = NULL; + into->name = steal(&from->name); } group_names_in_both = MIN(darray_size(into->group_names), @@ -579,8 +579,7 @@ HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *include) InitSymbolsInfo(&included, info->keymap, 0 /* unused */, info->actions, &info->mods); - included.name = include->stmt; - include->stmt = NULL; + included.name = steal(&include->stmt); for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) { SymbolsInfo next_incl; diff --git a/src/xkbcomp/types.c b/src/xkbcomp/types.c index f22c9ef07..404bfea28 100644 --- a/src/xkbcomp/types.c +++ b/src/xkbcomp/types.c @@ -31,6 +31,7 @@ #include "vmod.h" #include "expr.h" #include "include.h" +#include "util-mem.h" enum type_field { TYPE_FIELD_MASK = (1 << 0), @@ -192,8 +193,7 @@ MergeIncludedKeyTypes(KeyTypesInfo *into, KeyTypesInfo *from, into->mods = from->mods; if (into->name == NULL) { - into->name = from->name; - from->name = NULL; + into->name = steal(&from->name); } if (darray_empty(into->types)) { @@ -228,8 +228,7 @@ HandleIncludeKeyTypes(KeyTypesInfo *info, IncludeStmt *include) } InitKeyTypesInfo(&included, info->ctx, 0 /* unused */, &info->mods); - included.name = include->stmt; - include->stmt = NULL; + included.name = steal(&include->stmt); for (IncludeStmt *stmt = include; stmt; stmt = stmt->next_incl) { KeyTypesInfo next_incl;