diff mbox series

[v2,02/26] modpost: change mod->gpl_compatible to bool type

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

Commit Message

Masahiro Yamada May 1, 2022, 8:40 a.m. UTC
Currently, mod->gpl_compatible is tristate; it is set to -1 by default,
then to 1 or 0 when MODULE_LICENSE() is found.

Maybe, -1 was chosen to represent the 'unknown' license, but it is not
useful.

The current code:

    if (!mod->gpl_compatible)
            check_for_gpl_usage(exp->export, basename, exp->name);

... only cares whether gpl_compatible is zero or not.

Change it to a bool type with the initial value 'true', which has no
functional change.

The default value should be 'true' instead of 'false'.

Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into
error"), unknown module license is an error.

The error message, "missing MODULE_LICENSE()" is enough to explain the
issue. It is not sensible to show another message, "GPL-incompatible
module ... uses GPL-only symbol".

Add comments to explain this.

While I was here, I renamed gpl_compatible to is_gpl_compatible for
clarification, and also slightly refactored the code.

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

Changes in v2:
  - Split as a separate commit
  - More code refactoring
  - Add a comment
  - Rename it to ->is_gpl_compatible (Nick)

 scripts/mod/modpost.c | 17 +++++++++++------
 scripts/mod/modpost.h |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

Comments

Nick Desaulniers May 3, 2022, 9:45 p.m. UTC | #1
On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, mod->gpl_compatible is tristate; it is set to -1 by default,
> then to 1 or 0 when MODULE_LICENSE() is found.
>
> Maybe, -1 was chosen to represent the 'unknown' license, but it is not
> useful.
>
> The current code:
>
>     if (!mod->gpl_compatible)
>             check_for_gpl_usage(exp->export, basename, exp->name);
>
> ... only cares whether gpl_compatible is zero or not.
>
> Change it to a bool type with the initial value 'true', which has no
> functional change.
>
> The default value should be 'true' instead of 'false'.
>
> Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into
> error"), unknown module license is an error.
>
> The error message, "missing MODULE_LICENSE()" is enough to explain the
> issue. It is not sensible to show another message, "GPL-incompatible
> module ... uses GPL-only symbol".
>
> Add comments to explain this.
>
> While I was here, I renamed gpl_compatible to is_gpl_compatible for
> clarification, and also slightly refactored the code.
>
> 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 a6035ab78cd8..f2d6c152cd3f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -187,7 +187,14 @@  static struct module *new_module(const char *modname)
 	/* add to list */
 	strcpy(mod->name, modname);
 	mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
-	mod->gpl_compatible = -1;
+
+	/*
+	 * Set mod->is_gpl_compatible to true by default to skip
+	 * check_for_gpl_usage() when MODULE_LICENSE() is missing.
+	 * If it is the case, modpost will exit with error anyway.
+	 */
+	mod->is_gpl_compatible = true;
+
 	mod->next = modules;
 	modules = mod;
 
@@ -2012,10 +2019,8 @@  static void read_symbols(const char *modname)
 		if (!license)
 			error("missing MODULE_LICENSE() in %s\n", modname);
 		while (license) {
-			if (license_is_gpl_compatible(license))
-				mod->gpl_compatible = 1;
-			else {
-				mod->gpl_compatible = 0;
+			if (!license_is_gpl_compatible(license)) {
+				mod->is_gpl_compatible = false;
 				break;
 			}
 			license = get_next_modinfo(&info, "license", license);
@@ -2183,7 +2188,7 @@  static void check_exports(struct module *mod)
 			add_namespace(&mod->missing_namespaces, exp->namespace);
 		}
 
-		if (!mod->gpl_compatible)
+		if (!mod->is_gpl_compatible)
 			check_for_gpl_usage(exp->export, basename, exp->name);
 	}
 }
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 7ccfcc8899c1..f21a3782885b 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -110,7 +110,7 @@  buf_write(struct buffer *buf, const char *s, int len);
 
 struct module {
 	struct module *next;
-	int gpl_compatible;
+	bool is_gpl_compatible;
 	struct symbol *unres;
 	bool from_dump;		/* true if module was loaded from *.symvers */
 	bool is_vmlinux;