diff mbox series

[1/2] kbuild: use modpost-args-y to clean up modpost arguments

Message ID 20220906193436.2135744-1-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/2] kbuild: use modpost-args-y to clean up modpost arguments | expand

Commit Message

Masahiro Yamada Sept. 6, 2022, 7:34 p.m. UTC 
Separate the modpost executable (scripts/mod/modpost) and the command
parameters for the next commit.

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

 Makefile                 |  2 +-
 scripts/Makefile.modpost | 22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

Comments

Nick Desaulniers Sept. 7, 2022, 5:26 a.m. UTC | #1 
On Tue, Sep 6, 2022 at 12:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Separate the modpost executable (scripts/mod/modpost) and the command
> parameters for the next commit.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  Makefile                 |  2 +-
>  scripts/Makefile.modpost | 22 +++++++++++-----------
>  2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index aee4f0769d9d..e07236209606 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1908,7 +1908,7 @@ tags TAGS cscope gtags: FORCE
>  # ---------------------------------------------------------------------------
>
>  PHONY += nsdeps
> -nsdeps: export KBUILD_NSDEPS=1
> +nsdeps: export KBUILD_NSDEPS=y
>  nsdeps: modules
>         $(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
>
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index ceb1d78140e7..87821a5fe16b 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -38,17 +38,17 @@ __modpost:
>  include include/config/auto.conf
>  include $(srctree)/scripts/Kbuild.include
>
> -MODPOST = scripts/mod/modpost                                                          \
> -       $(if $(CONFIG_MODVERSIONS),-m)                                                  \
> -       $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)                                        \
> -       $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)                                  \
> -       $(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))                                     \
> -       $(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N)       \
> -       -o $@
> +MODPOST = scripts/mod/modpost
> +
> +modpost-args-$(CONFIG_MODVERSIONS)                             += -m
> +modpost-args-$(CONFIG_MODULE_SRCVERSION_ALL)                   += -a
> +modpost-args-$(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)  += -N
> +modpost-args-$(KBUILD_NSDEPS)                                  += -N -d $(MODULES_NSDEPS)

Was it intentional that you included -N for KBUILD_NSDEPS? That seems
like a change in behavior.  Everything else LGTM but that.

> +modpost-args-y += $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)

Ugh, your pattern kind of falls apart for the above ^.  Maybe this is
the most concise way to express this, but boy I sure do sometimes wish
for a programming language that had booleans and simple negation.
Maybe I would regret that in a build system...
Masahiro Yamada Sept. 15, 2022, 6:07 a.m. UTC | #2 
On Wed, Sep 7, 2022 at 2:26 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Tue, Sep 6, 2022 at 12:35 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Separate the modpost executable (scripts/mod/modpost) and the command
> > parameters for the next commit.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  Makefile                 |  2 +-
> >  scripts/Makefile.modpost | 22 +++++++++++-----------
> >  2 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index aee4f0769d9d..e07236209606 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1908,7 +1908,7 @@ tags TAGS cscope gtags: FORCE
> >  # ---------------------------------------------------------------------------
> >
> >  PHONY += nsdeps
> > -nsdeps: export KBUILD_NSDEPS=1
> > +nsdeps: export KBUILD_NSDEPS=y
> >  nsdeps: modules
> >         $(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
> >
> > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> > index ceb1d78140e7..87821a5fe16b 100644
> > --- a/scripts/Makefile.modpost
> > +++ b/scripts/Makefile.modpost
> > @@ -38,17 +38,17 @@ __modpost:
> >  include include/config/auto.conf
> >  include $(srctree)/scripts/Kbuild.include
> >
> > -MODPOST = scripts/mod/modpost                                                          \
> > -       $(if $(CONFIG_MODVERSIONS),-m)                                                  \
> > -       $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)                                        \
> > -       $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)                                  \
> > -       $(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))                                     \
> > -       $(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N)       \
> > -       -o $@
> > +MODPOST = scripts/mod/modpost
> > +
> > +modpost-args-$(CONFIG_MODVERSIONS)                             += -m
> > +modpost-args-$(CONFIG_MODULE_SRCVERSION_ALL)                   += -a
> > +modpost-args-$(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)  += -N
> > +modpost-args-$(KBUILD_NSDEPS)                                  += -N -d $(MODULES_NSDEPS)
>
> Was it intentional that you included -N for KBUILD_NSDEPS?


Yes.
See $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS)
in the original code.


> That seems
> like a change in behavior.  Everything else LGTM but that.


No behavior change.


If both CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
and KBUILD_NSDEPS are defined, -N was previously passed
just once, but now twice.
This is a slight change, but behavior is still the same.





> > +modpost-args-y += $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)
>
> Ugh, your pattern kind of falls apart for the above ^.  Maybe this is
> the most concise way to express this, but boy I sure do sometimes wish
> for a programming language that had booleans and simple negation.
> Maybe I would regret that in a build system...
> --
> Thanks,
> ~Nick Desaulniers


I do not think CONFIG_SECTION_MISMATCH_WARN_ONLY will live long,
but a cleaner fix is to stop negating the option.

Anyway, I do not care about that.







diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 911606496341..c5a18920646d 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -44,7 +44,7 @@ include $(srctree)/scripts/Kbuild.include
 MODPOST = scripts/mod/modpost
                 \
        $(if $(CONFIG_MODVERSIONS),-m)
                 \
        $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)
                 \
-       $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)
                 \
+       $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),-E)
                 \
        -o $@

 ifdef MODPOST_VMLINUX
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2c80da0220c3..be55dd541055 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -33,7 +33,7 @@ static bool external_module;
 static bool warn_unresolved;

 static int sec_mismatch_count;
-static bool sec_mismatch_warn_only = true;
+static bool sec_mismatch_warn_only;
 /* ignore missing files */
 static bool ignore_missing_files;
 /* If set to 1, only warn (instead of error) about missing ns imports */
@@ -2355,7 +2355,7 @@ int main(int argc, char **argv)
                        warn_unresolved = true;
                        break;
                case 'E':
-                       sec_mismatch_warn_only = false;
+                       sec_mismatch_warn_only = true;
                        break;
                case 'N':
                        allow_missing_ns_imports = true;
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index aee4f0769d9d..e07236209606 100644
--- a/Makefile
+++ b/Makefile
@@ -1908,7 +1908,7 @@  tags TAGS cscope gtags: FORCE
 # ---------------------------------------------------------------------------
 
 PHONY += nsdeps
-nsdeps: export KBUILD_NSDEPS=1
+nsdeps: export KBUILD_NSDEPS=y
 nsdeps: modules
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
 
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index ceb1d78140e7..87821a5fe16b 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -38,17 +38,17 @@  __modpost:
 include include/config/auto.conf
 include $(srctree)/scripts/Kbuild.include
 
-MODPOST = scripts/mod/modpost								\
-	$(if $(CONFIG_MODVERSIONS),-m)							\
-	$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)					\
-	$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)					\
-	$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS))					\
-	$(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N)	\
-	-o $@
+MODPOST = scripts/mod/modpost
+
+modpost-args-$(CONFIG_MODVERSIONS)				+= -m
+modpost-args-$(CONFIG_MODULE_SRCVERSION_ALL)			+= -a
+modpost-args-$(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)	+= -N
+modpost-args-$(KBUILD_NSDEPS)					+= -N -d $(MODULES_NSDEPS)
+modpost-args-y += $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)
 
 # 'make -i -k' ignores compile errors, and builds as many modules as possible.
 ifneq ($(findstring i,$(filter-out --%,$(MAKEFLAGS))),)
-MODPOST += -n
+modpost-args-y += -n
 endif
 
 ifeq ($(KBUILD_EXTMOD),)
@@ -99,12 +99,12 @@  module.symvers-if-present := $(wildcard Module.symvers)
 output-symdump := $(KBUILD_EXTMOD)/Module.symvers
 missing-input := $(filter-out $(module.symvers-if-present), Module.symvers)
 
-MODPOST += -e $(addprefix -i ,$(module.symvers-if-present) $(KBUILD_EXTRA_SYMBOLS))
+modpost-args-y += -e $(addprefix -i ,$(module.symvers-if-present) $(KBUILD_EXTRA_SYMBOLS))
 
 endif # ($(KBUILD_EXTMOD),)
 
 ifneq ($(KBUILD_MODPOST_WARN)$(missing-input),)
-MODPOST += -w
+modpost-args-y += -w
 endif
 
 modorder-if-needed := $(if $(KBUILD_MODULES), $(MODORDER))
@@ -117,7 +117,7 @@  quiet_cmd_modpost = MODPOST $@
 		echo >&2 "WARNING: $(missing-input) is missing."; \
 		echo >&2 "         Modules may not have dependencies or modversions."; \
 		echo >&2 "         You may get many unresolved symbol warnings.";) \
-	sed 's/ko$$/o/' $(or $(modorder-if-needed), /dev/null) | $(MODPOST) $(vmlinux.o-if-present) -T -
+	sed 's/ko$$/o/' $(or $(modorder-if-needed), /dev/null) | $(MODPOST) $(modpost-args-y) $(vmlinux.o-if-present) -T - -o $@
 
 targets += $(output-symdump)
 $(output-symdump): $(modorder-if-needed) $(vmlinux.o-if-present) $(moudle.symvers-if-present) FORCE