From: Emil Velikov Date: Sat, 18 Jul 2026 23:36:55 +0000 (+0100) Subject: modinfo: rework parm handling X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4fc769e9d08eb4c3db2307176e472aaeb9d8a7ad;p=thirdparty%2Fkmod.git modinfo: rework parm handling Currently, we grow a list of parm entries and print each entry upon being added. In addition, we have two separate code-paths for printing depending if "--field parm" was provided on not. Swap that for sufficiently sized, pre-allocated, buffers and unify the print paths. Effectively fixing the broken `modinfo -0 modulename` output. v2: - Find matching or empty entry in a single loop (add_param) - match up-to UINT_MAX parm/parmtype entries - drop unneeded INT_MAX checks for strlen(value) - don't return success if we fail to [cm]alloc Signed-off-by: Emil Velikov Link: https://github.com/kmod-project/kmod/pull/451 Signed-off-by: Lucas De Marchi --- diff --git a/tools/modinfo.c b/tools/modinfo.c index b8485015..1ccdfaaf 100644 --- a/tools/modinfo.c +++ b/tools/modinfo.c @@ -26,7 +26,6 @@ static char separator = '\n'; static const char *field; struct param { - struct param *next; const char *name; const char *desc; const char *type; @@ -41,7 +40,7 @@ enum parm_info { }; static int add_param(const char *name, size_t namelen, enum parm_info parm_info, - const char *value, struct param **list) + const char *value, struct param *params, unsigned int params_count) { size_t valuelen = strlen(value); struct param *it; @@ -49,40 +48,36 @@ static int add_param(const char *name, size_t namelen, enum parm_info parm_info, if (namelen > INT_MAX || valuelen > INT_MAX) return -EINVAL; - for (it = *list; it != NULL; it = it->next) { - if (it->namelen == (int)namelen && memcmp(it->name, name, namelen) == 0) - break; - } + /* We are guaranteed to have a match, or at least one empty entry */ + for (unsigned int i = 0; i < params_count; i++) { + it = ¶ms[i]; + + if (it->name != NULL && (it->namelen != (int)namelen || + memcmp(it->name, name, namelen) != 0)) { + continue; + } - if (it == NULL) { - it = malloc(sizeof(struct param)); - if (it == NULL) - return -ENOMEM; - it->next = *list; - *list = it; it->name = name; it->namelen = namelen; - it->desc = NULL; - it->type = NULL; - it->desclen = 0; - it->typelen = 0; - } - switch (parm_info) { - case (parm_desc): - it->desc = value; - it->desclen = (int)valuelen; - break; - case (parm_type): - it->type = value; - it->typelen = (int)valuelen; + switch (parm_info) { + case (parm_desc): + it->desc = value; + it->desclen = (int)valuelen; + break; + case (parm_type): + it->type = value; + it->typelen = (int)valuelen; + break; + } break; } return 0; } -static int process_parm(enum parm_info parm_info, const char *value, struct param **params) +static int process_parm(enum parm_info parm_info, const char *value, struct param *params, + unsigned int params_count) { const char *name; size_t namelen; @@ -101,7 +96,7 @@ static int process_parm(enum parm_info parm_info, const char *value, struct para name = value; namelen = colon - value; - ret = add_param(name, namelen, parm_info, colon + 1, params); + ret = add_param(name, namelen, parm_info, colon + 1, params, params_count); if (ret < 0) { ERR("Unable to add parameter: %s\n", strerror(-ret)); return -ENOMEM; @@ -127,50 +122,6 @@ static void print_line(const char *key, const char *value) } } -static int modinfo_params_do(const struct kmod_list *list) -{ - const struct kmod_list *l; - struct param *params = NULL; - int err = 0; - - kmod_list_foreach(l, list) { - const char *key = kmod_module_info_get_key(l); - const char *value = kmod_module_info_get_value(l); - if (streq(key, "parm")) { - err = process_parm(parm_desc, value, ¶ms); - if (err < 0) - goto end; - } else if (streq(key, "parmtype")) { - err = process_parm(parm_type, value, ¶ms); - if (err < 0) - goto end; - } - } - - while (params != NULL) { - struct param *p = params; - params = p->next; - - if (p->type != NULL) - printf("%.*s:%.*s (%.*s)%c", p->namelen, p->name, p->desclen, - p->desc, p->typelen, p->type, separator); - else - printf("%.*s:%.*s%c", p->namelen, p->name, p->desclen, p->desc, - separator); - - free(p); - } - -end: - while (params != NULL) { - void *tmp = params; - params = params->next; - free(tmp); - } - - return err; -} - static int modinfo_do(struct kmod_module *mod) { const enum kmod_module_initstate state = kmod_module_get_initstate(mod); @@ -180,6 +131,8 @@ static int modinfo_do(struct kmod_module *mod) const bool print_parm = !print_all && streq(field, "parm"); struct kmod_list *l, *list = NULL; struct param *params = NULL; + unsigned int params_count = 0; + char *sbuf = NULL; int err; /* TODO: align builtin vs not wrt listing "name:" via kmod_module_get_info() */ @@ -214,16 +167,58 @@ static int modinfo_do(struct kmod_module *mod) return err; } - if (print_parm) { - err = modinfo_params_do(list); - goto end; + if (print_all || print_parm) { + size_t count = 0; + size_t longest_desc = 0; + size_t longest_type = 0; + + /* + * Usually parm/parmtype come in pairs, where we print once per pair. + * + * Get worst case scenario & longest entry, for sufficiently large buffers. + */ + kmod_list_foreach(l, list) { + const char *key = kmod_module_info_get_key(l); + + if (streq(key, "parm")) { + const char *value = kmod_module_info_get_value(l); + size_t len = strlen(value); + + count++; + + if (len > longest_desc) + longest_desc = len; + } else if (streq(key, "parmtype")) { + const char *value = kmod_module_info_get_value(l); + size_t len = strlen(value); + + count++; + + if (len > longest_type) + longest_type = len; + } + } + + /* XXX: do we want to emit a warning/error here? */ + params_count = (params_count > UINT_MAX) ? UINT_MAX : (unsigned int)count; + + params = calloc(params_count, sizeof(*params)); + /* + * The "name:" exists in both parm&parmtype, so don't worry if we + * overallocate. + */ + sbuf = malloc(longest_desc + longest_type + strlen(" ()")); + if (params == NULL || sbuf == NULL) { + err = -ENOMEM; + goto end; + } } kmod_list_foreach(l, list) { const char *key = kmod_module_info_get_key(l); const char *value = kmod_module_info_get_value(l); - if (!print_all) { + if (!print_all && !print_parm) { if (streq(field, key)) { print_line(NULL, value); goto end; @@ -231,41 +226,37 @@ static int modinfo_do(struct kmod_module *mod) continue; } if (streq(key, "parm")) { - err = process_parm(parm_desc, value, ¶ms); + err = process_parm(parm_desc, value, params, params_count); if (err < 0) goto end; } else if (streq(key, "parmtype")) { - err = process_parm(parm_type, value, ¶ms); + err = process_parm(parm_type, value, params, params_count); if (err < 0) goto end; } else { - print_line(key, value); + if (print_all) + print_line(key, value); } } - if (!print_all) - goto end; - - while (params != NULL) { - struct param *p = params; - params = p->next; + for (unsigned int i = 0; i < params_count; i++) { + struct param *p = ¶ms[i]; + if (p->name == NULL) + continue; if (p->type != NULL) - printf("%-16s%.*s:%.*s (%.*s)%c", "parm:", p->namelen, p->name, - p->desclen, p->desc, p->typelen, p->type, separator); + sprintf(sbuf, "%.*s:%.*s (%.*s)", p->namelen, p->name, p->desclen, + p->desc, p->typelen, p->type); else - printf("%-16s%.*s:%.*s%c", "parm:", p->namelen, p->name, - p->desclen, p->desc, separator); + sprintf(sbuf, "%.*s:%.*s", p->namelen, p->name, p->desclen, + p->desc); - free(p); + print_line(print_parm ? NULL : "parm", sbuf); } end: - while (params != NULL) { - void *tmp = params; - params = params->next; - free(tmp); - } + free(sbuf); + free(params); kmod_module_info_free_list(list); return err;