diff mbox series

[08/27] modpost: traverse modules in order

Message ID 20220424190811.1678416-9-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series kbuild: yet another series of cleanups (modpost and LTO) | expand

Commit Message

Masahiro Yamada April 24, 2022, 7:07 p.m. UTC
Currently, modpost manages modules in a singly liked list; it adds a new
node to the head, and traverses the list from new to old.

It works, but the error messages are shown in the reverse order.

If you have a Makefile like this:

  obj-m += foo.o bar.o

then, modpost shows error messages in bar.o, foo.o, in this order.

Use a doubly linked list to keep the order in modules.order; use
list_add_tail() for the node addition and list_for_each_entry() for
the list traverse.

Now that the kernel's list macros have been imported to modpost, I will
use them actively going forward.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 18 +++++++++---------
 scripts/mod/modpost.h |  3 ++-
 2 files changed, 11 insertions(+), 10 deletions(-)

Comments

Nick Desaulniers April 26, 2022, 4:49 p.m. UTC | #1
On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, modpost manages modules in a singly liked list; it adds a new

s/liked/linked/

> node to the head, and traverses the list from new to old.
>
> It works, but the error messages are shown in the reverse order.
>
> If you have a Makefile like this:
>
>   obj-m += foo.o bar.o
>
> then, modpost shows error messages in bar.o, foo.o, in this order.
>
> Use a doubly linked list to keep the order in modules.order; use
> list_add_tail() for the node addition and list_for_each_entry() for
> the list traverse.
>
> Now that the kernel's list macros have been imported to modpost, I will
> use them actively going forward.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
diff mbox series

Patch

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 52dd07a36379..86416e4af626 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -165,16 +165,17 @@  char *get_line(char **stringp)
 }
 
 /* A list of all modules we processed */
-static struct module *modules;
+LIST_HEAD(modules);
 
 static struct module *find_module(const char *modname)
 {
 	struct module *mod;
 
-	for (mod = modules; mod; mod = mod->next)
+	list_for_each_entry(mod, &modules, list) {
 		if (strcmp(mod->name, modname) == 0)
-			break;
-	return mod;
+			return mod;
+	}
+	return NULL;
 }
 
 static struct module *new_module(const char *modname)
@@ -184,12 +185,11 @@  static struct module *new_module(const char *modname)
 	mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1));
 	memset(mod, 0, sizeof(*mod));
 
-	/* add to list */
 	strcpy(mod->name, modname);
 	mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
 	mod->gpl_compatible = true;
-	mod->next = modules;
-	modules = mod;
+
+	list_add_tail(&mod->list, &modules);
 
 	return mod;
 }
@@ -2467,7 +2467,7 @@  static void write_namespace_deps_files(const char *fname)
 	struct namespace_list *ns;
 	struct buffer ns_deps_buf = {};
 
-	for (mod = modules; mod; mod = mod->next) {
+	list_for_each_entry(mod, &modules, list) {
 
 		if (mod->from_dump || !mod->missing_namespaces)
 			continue;
@@ -2558,7 +2558,7 @@  int main(int argc, char **argv)
 	if (files_source)
 		read_symbols_from_files(files_source);
 
-	for (mod = modules; mod; mod = mod->next) {
+	list_for_each_entry(mod, &modules, list) {
 		char fname[PATH_MAX];
 		int ret;
 
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 4085bf5b33aa..c3b5d2f0e2bb 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -11,6 +11,7 @@ 
 #include <unistd.h>
 #include <elf.h>
 
+#include "list.h"
 #include "elfconfig.h"
 
 /* On BSD-alike OSes elf.h defines these according to host's word size */
@@ -114,7 +115,7 @@  struct namespace_list {
 };
 
 struct module {
-	struct module *next;
+	struct list_head list;
 	int gpl_compatible;
 	struct symbol *unres;
 	bool from_dump;		/* true if module was loaded from *.symvers */