On Mon, 30 Dec 2024, Tomas Paukrt wrote:
GCC 14.2 complains that the function filename2modname may return the address of
the local variable local_modname. It is a false positive, but still it is better
to allocate the memory directly to reduce the size of this function.
Signed-off-by: Tomas Paukrt <[email protected]>
---
modutils/modutils.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/modutils/modutils.c b/modutils/modutils.c
index cbff209..8be0378 100644
--- a/modutils/modutils.c
+++ b/modutils/modutils.c
@@ -94,14 +94,13 @@ int FAST_FUNC string_to_llist(char *string, llist_t
**llist, const char *delim)
char* FAST_FUNC filename2modname(const char *filename, char *modname)
{
- char local_modname[MODULE_NAME_LEN];
int i;
const char *from;
if (filename == NULL)
return NULL;
if (modname == NULL)
- modname = local_modname;
+ modname = xmalloc(MODULE_NAME_LEN);
// Disabled since otherwise "modprobe dir/name" would work
// as if it is "modprobe name". It is unclear why
// 'basenamization' was here in the first place.
@@ -111,9 +110,6 @@ char* FAST_FUNC filename2modname(const char *filename, char
*modname)
modname[i] = (from[i] == '-') ? '_' : from[i];
modname[i] = '\0';
- if (modname == local_modname)
- return xstrdup(modname);
-
return modname;
}
The above turns out to be (partial) revert of
https://git.busybox.net/busybox/commit/?id=cc70b6f8b6c6441e1c48690c7885700a2d389946
which was about moving a buffer out of depmod, later merged into common code.
There is only one caller (modprobe) that passes NULL modname,
so we could expand this it into an even more aggressive fix, where
filename2modname() requires a caller-supplied return buffer, and that
one caller does the xstrdup().
Also, the handling of filename == NULL was hiding a possible bug related
to a case in modprobe where the "alias" token is followed by nothing.
The prototype is now "char modname[static MODULE_NAME_LEN]", but I don't
know how people feel about that. Its C99, but I don't see [static...] being
used anywhere else in busybox?
d
==================
diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 77e42e3fb..3f22cd938 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -195,7 +195,7 @@ static void replace(char *s, char what, char with)
}
}
-static char *filename2modname(const char *filename, char *modname)
+static void filename2modname(const char *filename, char modname[static
MODULE_NAME_LEN])
{
int i;
const char *from;
@@ -208,8 +208,6 @@ static char *filename2modname(const char *filename, char
*modname)
for (i = 0; i < (MODULE_NAME_LEN-1) && from[i] != '\0' && from[i] !=
'.'; i++)
modname[i] = (from[i] == '-') ? '_' : from[i];
modname[i] = '\0';
-
- return modname;
}
static int pathname_matches_modname(const char *pathname, const char *modname)
diff --git a/modutils/modprobe.c b/modutils/modprobe.c
index 543f53e99..a3ec7d896 100644
--- a/modutils/modprobe.c
+++ b/modutils/modprobe.c
@@ -282,15 +282,18 @@ static int FAST_FUNC config_file_action(struct
recursive_state *state,
char wildcard[MODULE_NAME_LEN];
char *rmod;
- if (tokens[2] == NULL)
+ if (tokens[1] == NULL || tokens[2] == NULL)
continue;
filename2modname(tokens[1], wildcard);
for (l = G.probes; l; l = l->link) {
+ char rmodname[MODULE_NAME_LEN];
+
m = (struct module_entry *) l->data;
if (fnmatch(wildcard, m->modname, 0) != 0)
continue;
- rmod = filename2modname(tokens[2], NULL);
+ filename2modname(tokens[2], rmodname);
+ rmod = xstrdup(rmodname);
llist_add_to(&m->realnames, rmod);
if (m->flags & MODULE_FLAG_NEED_DEPS) {
diff --git a/modutils/modutils.c b/modutils/modutils.c
index cbff20961..d158504d0 100644
--- a/modutils/modutils.c
+++ b/modutils/modutils.c
@@ -92,16 +92,11 @@ int FAST_FUNC string_to_llist(char *string, llist_t
**llist, const char *delim)
return len;
}
-char* FAST_FUNC filename2modname(const char *filename, char *modname)
+void FAST_FUNC filename2modname(const char *filename, char modname[static
MODULE_NAME_LEN])
{
- char local_modname[MODULE_NAME_LEN];
int i;
const char *from;
- if (filename == NULL)
- return NULL;
- if (modname == NULL)
- modname = local_modname;
// Disabled since otherwise "modprobe dir/name" would work
// as if it is "modprobe name". It is unclear why
// 'basenamization' was here in the first place.
@@ -110,11 +105,6 @@ char* FAST_FUNC filename2modname(const char *filename,
char *modname)
for (i = 0; i < (MODULE_NAME_LEN-1) && from[i] != '\0' && from[i] !=
'.'; i++)
modname[i] = (from[i] == '-') ? '_' : from[i];
modname[i] = '\0';
-
- if (modname == local_modname)
- return xstrdup(modname);
-
- return modname;
}
#if ENABLE_FEATURE_CMDLINE_MODULE_OPTIONS
diff --git a/modutils/modutils.h b/modutils/modutils.h
index 4a702e97c..82f9dac79 100644
--- a/modutils/modutils.h
+++ b/modutils/modutils.h
@@ -49,7 +49,7 @@ void moddb_free(module_db *db) FAST_FUNC;
void replace(char *s, char what, char with) FAST_FUNC;
int string_to_llist(char *string, llist_t **llist, const char *delim)
FAST_FUNC;
-char *filename2modname(const char *filename, char *modname) FAST_FUNC;
+void filename2modname(const char *filename, char modname[static
MODULE_NAME_LEN]) FAST_FUNC;
#if ENABLE_FEATURE_CMDLINE_MODULE_OPTIONS
char *parse_cmdline_module_options(char **argv, int quote_spaces) FAST_FUNC;
#else
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
