The nsdeps script calls generate_deps() for every module in the
modules.order file. It prepends $objtree to obtain the full path of the
top-level modules.order file. This produces incorrect results when
calling nsdeps for an external module, as only the ns dependencies for
in-tree modules listed in $objtree/modules.order are generated rather
than the ns dependencies for the external module. To fix this, just use
the MODORDER variable provided by kbuild - it uses the correct path for
the relevant modules.order file (either in-tree or the one produced by
the external module build).
Signed-off-by: Jessica Yu <[email protected]>
---
So, not being too familiar with kbuild, I am not sure if MODORDER was the
appropriate kbuild variable to use, but I could not find anything else that
gives us the modules.order path. Masahiro, please let me know if this is
appropriate usage.
scripts/nsdeps | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/nsdeps b/scripts/nsdeps
index dda6fbac016e..54d2ab8f9e5c 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -52,7 +52,7 @@ generate_deps() {
done
}
-for f in `cat $objtree/modules.order`; do
+for f in `cat $MODORDER`; do
generate_deps $f
done
--
2.16.4
When adding or removing namespaces, calling `make` does not necessarily
remove existing stale .ns_deps files. That is, one could remove a
namespace, call make, and while modpost writes the correct, new .ns_deps
files, stale ones are not removed from the source tree, thus producing
incorrect results when running `make nsdeps`, i.e., inserting
MODULE_IMPORT_NS() statements for namespaces that have been removed.
Clean up old .ns_deps files before generating new ones with modpost.
Signed-off-by: Jessica Yu <[email protected]>
---
Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Makefile b/Makefile
index ffd7a912fc46..22f9894b346b 100644
--- a/Makefile
+++ b/Makefile
@@ -1685,6 +1685,8 @@ tags TAGS cscope gtags: FORCE
PHONY += nsdeps
nsdeps: modules
+ @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \
+ -name '*.ns_deps' -type f -print | xargs rm -f
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@
--
2.16.4
The nsdeps script passes a list of the module source files to
generate_deps_for_ns() as a space delimited string named $mod_source_files,
which then passes it to spatch. But since $mod_source_files is not encased
in quotes, each source file in that string is treated as a separate shell
function argument (as $2, $3, $4, etc.). However, the spatch invocation
only refers to $2, so only the first file out of $mod_source_files is
processed by spatch.
This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
get inserted) when a module is composed of many source files and the
"main" module file containing the MODULE_LICENSE() statement is not the
first file listed in $mod_source_files. Fix this by encasing
$mod_source_files in quotes so that the entirety of the string is
treated as a single argument and can be referred to as $2.
Signed-off-by: Jessica Yu <[email protected]>
---
scripts/nsdeps | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/nsdeps b/scripts/nsdeps
index 9ddcd5cb96b1..5055b059a81b 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -36,7 +36,7 @@ generate_deps() {
| sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
for ns in `cat $ns_deps_file`; do
echo "Adding namespace $ns to module $mod_name (if needed)."
- generate_deps_for_ns $ns $mod_source_files
+ generate_deps_for_ns $ns "$mod_source_files"
# sort the imports
for source_file in $mod_source_files; do
sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
--
2.16.4
On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
>
> When adding or removing namespaces, calling `make` does not necessarily
> remove existing stale .ns_deps files. That is, one could remove a
> namespace, call make, and while modpost writes the correct, new .ns_deps
> files, stale ones are not removed from the source tree, thus producing
> incorrect results when running `make nsdeps`, i.e., inserting
> MODULE_IMPORT_NS() statements for namespaces that have been removed.
> Clean up old .ns_deps files before generating new ones with modpost.
Hmm, correct.
But, removing *.ns_deps every time is not an elegant solution.
I want to try a different approach.
>
> Signed-off-by: Jessica Yu <[email protected]>
> ---
> Makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index ffd7a912fc46..22f9894b346b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1685,6 +1685,8 @@ tags TAGS cscope gtags: FORCE
> PHONY += nsdeps
>
> nsdeps: modules
> + @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \
> + -name '*.ns_deps' -type f -print | xargs rm -f
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps
> $(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@
>
> --
> 2.16.4
>
--
Best Regards
Masahiro Yamada
On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
>
> The nsdeps script passes a list of the module source files to
> generate_deps_for_ns() as a space delimited string named $mod_source_files,
> which then passes it to spatch. But since $mod_source_files is not encased
> in quotes, each source file in that string is treated as a separate shell
> function argument (as $2, $3, $4, etc.). However, the spatch invocation
> only refers to $2, so only the first file out of $mod_source_files is
> processed by spatch.
>
> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
> get inserted) when a module is composed of many source files and the
> "main" module file containing the MODULE_LICENSE() statement is not the
> first file listed in $mod_source_files. Fix this by encasing
> $mod_source_files in quotes so that the entirety of the string is
> treated as a single argument and can be referred to as $2.
>
> Signed-off-by: Jessica Yu <[email protected]>
> ---
> scripts/nsdeps | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index 9ddcd5cb96b1..5055b059a81b 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -36,7 +36,7 @@ generate_deps() {
> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
> for ns in `cat $ns_deps_file`; do
> echo "Adding namespace $ns to module $mod_name (if needed)."
> - generate_deps_for_ns $ns $mod_source_files
> + generate_deps_for_ns $ns "$mod_source_files"
> # sort the imports
> for source_file in $mod_source_files; do
> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
I think this change is correct, but
did you succeed in nsdeps for composite modules
with this patch only?
I think the following is needed too:
diff --git a/scripts/nsdeps b/scripts/nsdeps
index dda6fbac016e..5a23ea616446 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -31,9 +31,9 @@ generate_deps() {
local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
if [ ! -f "$ns_deps_file" ]; then return; fi
- local mod_source_files=`cat $mod_file | sed -n 1p \
+ local mod_source_files="`cat $mod_file | sed -n 1p
\
| sed -e 's/\.o/\.c/g' \
- | sed "s|[^ ]* *|${srctree}/&|g"`
+ | sed "s|[^ ]* *|${srctree}/&|g"`"
for ns in `cat $ns_deps_file`; do
echo "Adding namespace $ns to module $mod_name (if needed)."
generate_deps_for_ns $ns $mod_source_files
Without this, a module that consists of two files
will be expanded to:
local mod_source_files=source1.c source2.c
--
Best Regards
Masahiro Yamada
On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
>
> The nsdeps script calls generate_deps() for every module in the
> modules.order file. It prepends $objtree to obtain the full path of the
> top-level modules.order file. This produces incorrect results when
> calling nsdeps for an external module, as only the ns dependencies for
> in-tree modules listed in $objtree/modules.order are generated rather
> than the ns dependencies for the external module. To fix this, just use
> the MODORDER variable provided by kbuild - it uses the correct path for
> the relevant modules.order file (either in-tree or the one produced by
> the external module build).
>
> Signed-off-by: Jessica Yu <[email protected]>
> ---
>
> So, not being too familiar with kbuild, I am not sure if MODORDER was the
> appropriate kbuild variable to use, but I could not find anything else that
> gives us the modules.order path. Masahiro, please let me know if this is
> appropriate usage.
Right, MODORDER allows you to get access to
the right modules.order depending on what you are building
although this patch alone is not useful since nsdeps does not work with
external modules.
> scripts/nsdeps | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index dda6fbac016e..54d2ab8f9e5c 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -52,7 +52,7 @@ generate_deps() {
> done
> }
>
> -for f in `cat $objtree/modules.order`; do
> +for f in `cat $MODORDER`; do
> generate_deps $f
> done
>
> --
> 2.16.4
>
--
Best Regards
Masahiro Yamada
+++ Masahiro Yamada [29/10/19 21:57 +0900]:
>On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
>>
>> The nsdeps script passes a list of the module source files to
>> generate_deps_for_ns() as a space delimited string named $mod_source_files,
>> which then passes it to spatch. But since $mod_source_files is not encased
>> in quotes, each source file in that string is treated as a separate shell
>> function argument (as $2, $3, $4, etc.). However, the spatch invocation
>> only refers to $2, so only the first file out of $mod_source_files is
>> processed by spatch.
>>
>> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
>> get inserted) when a module is composed of many source files and the
>> "main" module file containing the MODULE_LICENSE() statement is not the
>> first file listed in $mod_source_files. Fix this by encasing
>> $mod_source_files in quotes so that the entirety of the string is
>> treated as a single argument and can be referred to as $2.
>>
>> Signed-off-by: Jessica Yu <[email protected]>
>> ---
>> scripts/nsdeps | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/nsdeps b/scripts/nsdeps
>> index 9ddcd5cb96b1..5055b059a81b 100644
>> --- a/scripts/nsdeps
>> +++ b/scripts/nsdeps
>> @@ -36,7 +36,7 @@ generate_deps() {
>> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
>> for ns in `cat $ns_deps_file`; do
>> echo "Adding namespace $ns to module $mod_name (if needed)."
>> - generate_deps_for_ns $ns $mod_source_files
>> + generate_deps_for_ns $ns "$mod_source_files"
>> # sort the imports
>> for source_file in $mod_source_files; do
>> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
>
>I think this change is correct, but
>did you succeed in nsdeps for composite modules
>with this patch only?
>
>I think the following is needed too:
>
>
>diff --git a/scripts/nsdeps b/scripts/nsdeps
>index dda6fbac016e..5a23ea616446 100644
>--- a/scripts/nsdeps
>+++ b/scripts/nsdeps
>@@ -31,9 +31,9 @@ generate_deps() {
> local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
> local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
> if [ ! -f "$ns_deps_file" ]; then return; fi
>- local mod_source_files=`cat $mod_file | sed -n 1p \
>+ local mod_source_files="`cat $mod_file | sed -n 1p
> \
> | sed -e 's/\.o/\.c/g' \
>- | sed "s|[^ ]* *|${srctree}/&|g"`
>+ | sed "s|[^ ]* *|${srctree}/&|g"`"
> for ns in `cat $ns_deps_file`; do
> echo "Adding namespace $ns to module $mod_name (if needed)."
> generate_deps_for_ns $ns $mod_source_files
>
>
>Without this, a module that consists of two files
>will be expanded to:
>
>local mod_source_files=source1.c source2.c
Yes, I was able to have nsdeps work for composite modules with just my
patch. Without this patch applied, the script produces the following
expansion of the generate_deps_for_ns call, (I just added a test
namespace MODULE):
Adding namespace MODULE to module fs/nfs/nfs.ko.
+ generate_deps_for_ns MODULE /tmp/ppyu/linux/fs/nfs/client.c /tmp/ppyu/linux/fs/nfs/dir.c /tmp/ppyu/linux/fs/nfs/file.c /tmp/ppyu/linux/fs/nfs/getroot.c /tmp/ppyu/linux/fs/nfs/inode.c /tmp/ppyu/linux/fs/nfs/super.c /tmp/ppyu/linux/fs/nfs/io.c /tmp/ppyu/linux/fs/nfs/direct.c /tmp/ppyu/linux/fs/nfs/pagelist.c /tmp/ppyu/linux/fs/nfs/read.c /tmp/ppyu/linux/fs/nfs/symlink.c /tmp/ppyu/linux/fs/nfs/unlink.c /tmp/ppyu/linux/fs/nfs/write.c /tmp/ppyu/linux/fs/nfs/namespace.c /tmp/ppyu/linux/fs/nfs/mount_clnt.c /tmp/ppyu/linux/fs/nfs/nfstrace.c /tmp/ppyu/linux/fs/nfs/export.c /tmp/ppyu/linux/fs/nfs/sysfs.c /tmp/ppyu/linux/fs/nfs/sysctl.c /tmp/ppyu/linux/fs/nfs/fscache.c /tmp/ppyu/linux/fs/nfs/fscache-index.c
+ /usr/bin/spatch --very-quiet --in-place --sp-file /tmp/ppyu/linux/scripts/coccinelle/misc/add_namespace.cocci -D ns=MODULE /tmp/ppyu/linux/fs/nfs/client.c
So only the first file got included in the spatch invocation. But the
spatch call gets fixed with all the files when quotes are added in the
call to generate_deps_for_ns.
But we need to include your change anyway, to make the script more
robust. It would probably prevent more shell script related bugs in
the future (Like [1]). I can respin this patch only while the other
ones are superceded by your patchset.
[1] https://unix.stackexchange.com/a/131767
On Thu, Oct 31, 2019 at 1:17 AM Jessica Yu <[email protected]> wrote:
>
> +++ Masahiro Yamada [29/10/19 21:57 +0900]:
> >On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
> >>
> >> The nsdeps script passes a list of the module source files to
> >> generate_deps_for_ns() as a space delimited string named $mod_source_files,
> >> which then passes it to spatch. But since $mod_source_files is not encased
> >> in quotes, each source file in that string is treated as a separate shell
> >> function argument (as $2, $3, $4, etc.). However, the spatch invocation
> >> only refers to $2, so only the first file out of $mod_source_files is
> >> processed by spatch.
> >>
> >> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
> >> get inserted) when a module is composed of many source files and the
> >> "main" module file containing the MODULE_LICENSE() statement is not the
> >> first file listed in $mod_source_files. Fix this by encasing
> >> $mod_source_files in quotes so that the entirety of the string is
> >> treated as a single argument and can be referred to as $2.
> >>
> >> Signed-off-by: Jessica Yu <[email protected]>
> >> ---
> >> scripts/nsdeps | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/nsdeps b/scripts/nsdeps
> >> index 9ddcd5cb96b1..5055b059a81b 100644
> >> --- a/scripts/nsdeps
> >> +++ b/scripts/nsdeps
> >> @@ -36,7 +36,7 @@ generate_deps() {
> >> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
> >> for ns in `cat $ns_deps_file`; do
> >> echo "Adding namespace $ns to module $mod_name (if needed)."
> >> - generate_deps_for_ns $ns $mod_source_files
> >> + generate_deps_for_ns $ns "$mod_source_files"
> >> # sort the imports
> >> for source_file in $mod_source_files; do
> >> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
> >
> >I think this change is correct, but
> >did you succeed in nsdeps for composite modules
> >with this patch only?
> >
> >I think the following is needed too:
> >
> >
> >diff --git a/scripts/nsdeps b/scripts/nsdeps
> >index dda6fbac016e..5a23ea616446 100644
> >--- a/scripts/nsdeps
> >+++ b/scripts/nsdeps
> >@@ -31,9 +31,9 @@ generate_deps() {
> > local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
> > local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
> > if [ ! -f "$ns_deps_file" ]; then return; fi
> >- local mod_source_files=`cat $mod_file | sed -n 1p \
> >+ local mod_source_files="`cat $mod_file | sed -n 1p
> > \
> > | sed -e 's/\.o/\.c/g' \
> >- | sed "s|[^ ]* *|${srctree}/&|g"`
> >+ | sed "s|[^ ]* *|${srctree}/&|g"`"
> > for ns in `cat $ns_deps_file`; do
> > echo "Adding namespace $ns to module $mod_name (if needed)."
> > generate_deps_for_ns $ns $mod_source_files
> >
> >
> >Without this, a module that consists of two files
> >will be expanded to:
> >
> >local mod_source_files=source1.c source2.c
>
> Yes, I was able to have nsdeps work for composite modules with just my
> patch. Without this patch applied, the script produces the following
> expansion of the generate_deps_for_ns call, (I just added a test
> namespace MODULE):
>
> Adding namespace MODULE to module fs/nfs/nfs.ko.
> + generate_deps_for_ns MODULE /tmp/ppyu/linux/fs/nfs/client.c /tmp/ppyu/linux/fs/nfs/dir.c /tmp/ppyu/linux/fs/nfs/file.c /tmp/ppyu/linux/fs/nfs/getroot.c /tmp/ppyu/linux/fs/nfs/inode.c /tmp/ppyu/linux/fs/nfs/super.c /tmp/ppyu/linux/fs/nfs/io.c /tmp/ppyu/linux/fs/nfs/direct.c /tmp/ppyu/linux/fs/nfs/pagelist.c /tmp/ppyu/linux/fs/nfs/read.c /tmp/ppyu/linux/fs/nfs/symlink.c /tmp/ppyu/linux/fs/nfs/unlink.c /tmp/ppyu/linux/fs/nfs/write.c /tmp/ppyu/linux/fs/nfs/namespace.c /tmp/ppyu/linux/fs/nfs/mount_clnt.c /tmp/ppyu/linux/fs/nfs/nfstrace.c /tmp/ppyu/linux/fs/nfs/export.c /tmp/ppyu/linux/fs/nfs/sysfs.c /tmp/ppyu/linux/fs/nfs/sysctl.c /tmp/ppyu/linux/fs/nfs/fscache.c /tmp/ppyu/linux/fs/nfs/fscache-index.c
> + /usr/bin/spatch --very-quiet --in-place --sp-file /tmp/ppyu/linux/scripts/coccinelle/misc/add_namespace.cocci -D ns=MODULE /tmp/ppyu/linux/fs/nfs/client.c
>
> So only the first file got included in the spatch invocation. But the
> spatch call gets fixed with all the files when quotes are added in the
> call to generate_deps_for_ns.
>
> But we need to include your change anyway, to make the script more
> robust.
Hmm.
With this patch only, I see "bad variable name" error.
masahiro@grover:~/ref/linux$ make -j8 nsdeps
DESCEND objtool
CALL scripts/atomic/check-atomics.sh
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
Building modules, stage 2.
MODPOST 20 modules
WARNING: module nfs uses symbol foo from namespace USB_STORAGE, but
does not import it.
Building modules, stage 2.
MODPOST 20 modules
./scripts/nsdeps: 34: local: ./fs/nfs/dir.c: bad variable name
make: *** [Makefile;1689: nsdeps] Error 2
> It would probably prevent more shell script related bugs in
> the future (Like [1]). I can respin this patch only while the other
> ones are superceded by your patchset.
>
> [1] https://unix.stackexchange.com/a/131767
Anyway.
Is this patch aiming for v5.4 (i.e. fixes) or v5.5-rc1 ?
If you touch the mod_source_files line,
we will have a conflict because
https://patchwork.kernel.org/patch/11217839/
is touching the same line.
How should we organize the patch order?
--
Best Regards
Masahiro Yamada
+++ Masahiro Yamada [31/10/19 21:27 +0900]:
>On Thu, Oct 31, 2019 at 1:17 AM Jessica Yu <[email protected]> wrote:
>>
>> +++ Masahiro Yamada [29/10/19 21:57 +0900]:
>> >On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
>> >>
>> >> The nsdeps script passes a list of the module source files to
>> >> generate_deps_for_ns() as a space delimited string named $mod_source_files,
>> >> which then passes it to spatch. But since $mod_source_files is not encased
>> >> in quotes, each source file in that string is treated as a separate shell
>> >> function argument (as $2, $3, $4, etc.). However, the spatch invocation
>> >> only refers to $2, so only the first file out of $mod_source_files is
>> >> processed by spatch.
>> >>
>> >> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
>> >> get inserted) when a module is composed of many source files and the
>> >> "main" module file containing the MODULE_LICENSE() statement is not the
>> >> first file listed in $mod_source_files. Fix this by encasing
>> >> $mod_source_files in quotes so that the entirety of the string is
>> >> treated as a single argument and can be referred to as $2.
>> >>
>> >> Signed-off-by: Jessica Yu <[email protected]>
>> >> ---
>> >> scripts/nsdeps | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/scripts/nsdeps b/scripts/nsdeps
>> >> index 9ddcd5cb96b1..5055b059a81b 100644
>> >> --- a/scripts/nsdeps
>> >> +++ b/scripts/nsdeps
>> >> @@ -36,7 +36,7 @@ generate_deps() {
>> >> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
>> >> for ns in `cat $ns_deps_file`; do
>> >> echo "Adding namespace $ns to module $mod_name (if needed)."
>> >> - generate_deps_for_ns $ns $mod_source_files
>> >> + generate_deps_for_ns $ns "$mod_source_files"
>> >> # sort the imports
>> >> for source_file in $mod_source_files; do
>> >> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
>> >
>> >I think this change is correct, but
>> >did you succeed in nsdeps for composite modules
>> >with this patch only?
>> >
>> >I think the following is needed too:
>> >
>> >
>> >diff --git a/scripts/nsdeps b/scripts/nsdeps
>> >index dda6fbac016e..5a23ea616446 100644
>> >--- a/scripts/nsdeps
>> >+++ b/scripts/nsdeps
>> >@@ -31,9 +31,9 @@ generate_deps() {
>> > local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
>> > local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
>> > if [ ! -f "$ns_deps_file" ]; then return; fi
>> >- local mod_source_files=`cat $mod_file | sed -n 1p \
>> >+ local mod_source_files="`cat $mod_file | sed -n 1p
>> > \
>> > | sed -e 's/\.o/\.c/g' \
>> >- | sed "s|[^ ]* *|${srctree}/&|g"`
>> >+ | sed "s|[^ ]* *|${srctree}/&|g"`"
>> > for ns in `cat $ns_deps_file`; do
>> > echo "Adding namespace $ns to module $mod_name (if needed)."
>> > generate_deps_for_ns $ns $mod_source_files
>> >
>> >
>> >Without this, a module that consists of two files
>> >will be expanded to:
>> >
>> >local mod_source_files=source1.c source2.c
>>
>> Yes, I was able to have nsdeps work for composite modules with just my
>> patch. Without this patch applied, the script produces the following
>> expansion of the generate_deps_for_ns call, (I just added a test
>> namespace MODULE):
>>
>> Adding namespace MODULE to module fs/nfs/nfs.ko.
>> + generate_deps_for_ns MODULE /tmp/ppyu/linux/fs/nfs/client.c /tmp/ppyu/linux/fs/nfs/dir.c /tmp/ppyu/linux/fs/nfs/file.c /tmp/ppyu/linux/fs/nfs/getroot.c /tmp/ppyu/linux/fs/nfs/inode.c /tmp/ppyu/linux/fs/nfs/super.c /tmp/ppyu/linux/fs/nfs/io.c /tmp/ppyu/linux/fs/nfs/direct.c /tmp/ppyu/linux/fs/nfs/pagelist.c /tmp/ppyu/linux/fs/nfs/read.c /tmp/ppyu/linux/fs/nfs/symlink.c /tmp/ppyu/linux/fs/nfs/unlink.c /tmp/ppyu/linux/fs/nfs/write.c /tmp/ppyu/linux/fs/nfs/namespace.c /tmp/ppyu/linux/fs/nfs/mount_clnt.c /tmp/ppyu/linux/fs/nfs/nfstrace.c /tmp/ppyu/linux/fs/nfs/export.c /tmp/ppyu/linux/fs/nfs/sysfs.c /tmp/ppyu/linux/fs/nfs/sysctl.c /tmp/ppyu/linux/fs/nfs/fscache.c /tmp/ppyu/linux/fs/nfs/fscache-index.c
>> + /usr/bin/spatch --very-quiet --in-place --sp-file /tmp/ppyu/linux/scripts/coccinelle/misc/add_namespace.cocci -D ns=MODULE /tmp/ppyu/linux/fs/nfs/client.c
>>
>> So only the first file got included in the spatch invocation. But the
>> spatch call gets fixed with all the files when quotes are added in the
>> call to generate_deps_for_ns.
>>
>> But we need to include your change anyway, to make the script more
>> robust.
>
>Hmm.
>With this patch only, I see "bad variable name" error.
>
>
>
>masahiro@grover:~/ref/linux$ make -j8 nsdeps
> DESCEND objtool
> CALL scripts/atomic/check-atomics.sh
> CALL scripts/checksyscalls.sh
> CHK include/generated/compile.h
> Building modules, stage 2.
> MODPOST 20 modules
>WARNING: module nfs uses symbol foo from namespace USB_STORAGE, but
>does not import it.
> Building modules, stage 2.
> MODPOST 20 modules
>./scripts/nsdeps: 34: local: ./fs/nfs/dir.c: bad variable name
>make: *** [Makefile;1689: nsdeps] Error 2
Hm, I was having trouble reproducing this until I changed the shell to
dash, /bin/sh is a symlink to bash on my system, that might explain
slightly different behavior. In any case, we should add quotes in both
places.
>> It would probably prevent more shell script related bugs in
>> the future (Like [1]). I can respin this patch only while the other
>> ones are superceded by your patchset.
>>
>> [1] https://unix.stackexchange.com/a/131767
>
>Anyway.
>
>Is this patch aiming for v5.4 (i.e. fixes) or v5.5-rc1 ?
I am hoping for fixes, we should try to get all the small bugs out of
nsdeps by 5.4 if we can..
>If you touch the mod_source_files line,
>we will have a conflict because
>https://patchwork.kernel.org/patch/11217839/
>is touching the same line.
>
>How should we organize the patch order?
Would you like to fold these changes into your nsdeps improvements
patchset? Since it's a pretty trivial change.
Thanks!
On Thu, Oct 31, 2019 at 10:41 PM Jessica Yu <[email protected]> wrote:
>
> +++ Masahiro Yamada [31/10/19 21:27 +0900]:
> >On Thu, Oct 31, 2019 at 1:17 AM Jessica Yu <[email protected]> wrote:
> >>
> >> +++ Masahiro Yamada [29/10/19 21:57 +0900]:
> >> >On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
> >> >>
> >> >> The nsdeps script passes a list of the module source files to
> >> >> generate_deps_for_ns() as a space delimited string named $mod_source_files,
> >> >> which then passes it to spatch. But since $mod_source_files is not encased
> >> >> in quotes, each source file in that string is treated as a separate shell
> >> >> function argument (as $2, $3, $4, etc.). However, the spatch invocation
> >> >> only refers to $2, so only the first file out of $mod_source_files is
> >> >> processed by spatch.
> >> >>
> >> >> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
> >> >> get inserted) when a module is composed of many source files and the
> >> >> "main" module file containing the MODULE_LICENSE() statement is not the
> >> >> first file listed in $mod_source_files. Fix this by encasing
> >> >> $mod_source_files in quotes so that the entirety of the string is
> >> >> treated as a single argument and can be referred to as $2.
> >> >>
> >> >> Signed-off-by: Jessica Yu <[email protected]>
> >> >> ---
> >> >> scripts/nsdeps | 2 +-
> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/scripts/nsdeps b/scripts/nsdeps
> >> >> index 9ddcd5cb96b1..5055b059a81b 100644
> >> >> --- a/scripts/nsdeps
> >> >> +++ b/scripts/nsdeps
> >> >> @@ -36,7 +36,7 @@ generate_deps() {
> >> >> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
> >> >> for ns in `cat $ns_deps_file`; do
> >> >> echo "Adding namespace $ns to module $mod_name (if needed)."
> >> >> - generate_deps_for_ns $ns $mod_source_files
> >> >> + generate_deps_for_ns $ns "$mod_source_files"
> >> >> # sort the imports
> >> >> for source_file in $mod_source_files; do
> >> >> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
> >> >
> >> >I think this change is correct, but
> >> >did you succeed in nsdeps for composite modules
> >> >with this patch only?
> >> >
> >> >I think the following is needed too:
> >> >
> >> >
> >> >diff --git a/scripts/nsdeps b/scripts/nsdeps
> >> >index dda6fbac016e..5a23ea616446 100644
> >> >--- a/scripts/nsdeps
> >> >+++ b/scripts/nsdeps
> >> >@@ -31,9 +31,9 @@ generate_deps() {
> >> > local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
> >> > local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
> >> > if [ ! -f "$ns_deps_file" ]; then return; fi
> >> >- local mod_source_files=`cat $mod_file | sed -n 1p \
> >> >+ local mod_source_files="`cat $mod_file | sed -n 1p
> >> > \
> >> > | sed -e 's/\.o/\.c/g' \
> >> >- | sed "s|[^ ]* *|${srctree}/&|g"`
> >> >+ | sed "s|[^ ]* *|${srctree}/&|g"`"
> >> > for ns in `cat $ns_deps_file`; do
> >> > echo "Adding namespace $ns to module $mod_name (if needed)."
> >> > generate_deps_for_ns $ns $mod_source_files
> >> >
> >> >
> >> >Without this, a module that consists of two files
> >> >will be expanded to:
> >> >
> >> >local mod_source_files=source1.c source2.c
> >>
> >> Yes, I was able to have nsdeps work for composite modules with just my
> >> patch. Without this patch applied, the script produces the following
> >> expansion of the generate_deps_for_ns call, (I just added a test
> >> namespace MODULE):
> >>
> >> Adding namespace MODULE to module fs/nfs/nfs.ko.
> >> + generate_deps_for_ns MODULE /tmp/ppyu/linux/fs/nfs/client.c /tmp/ppyu/linux/fs/nfs/dir.c /tmp/ppyu/linux/fs/nfs/file.c /tmp/ppyu/linux/fs/nfs/getroot.c /tmp/ppyu/linux/fs/nfs/inode.c /tmp/ppyu/linux/fs/nfs/super.c /tmp/ppyu/linux/fs/nfs/io.c /tmp/ppyu/linux/fs/nfs/direct.c /tmp/ppyu/linux/fs/nfs/pagelist.c /tmp/ppyu/linux/fs/nfs/read.c /tmp/ppyu/linux/fs/nfs/symlink.c /tmp/ppyu/linux/fs/nfs/unlink.c /tmp/ppyu/linux/fs/nfs/write.c /tmp/ppyu/linux/fs/nfs/namespace.c /tmp/ppyu/linux/fs/nfs/mount_clnt.c /tmp/ppyu/linux/fs/nfs/nfstrace.c /tmp/ppyu/linux/fs/nfs/export.c /tmp/ppyu/linux/fs/nfs/sysfs.c /tmp/ppyu/linux/fs/nfs/sysctl.c /tmp/ppyu/linux/fs/nfs/fscache.c /tmp/ppyu/linux/fs/nfs/fscache-index.c
> >> + /usr/bin/spatch --very-quiet --in-place --sp-file /tmp/ppyu/linux/scripts/coccinelle/misc/add_namespace.cocci -D ns=MODULE /tmp/ppyu/linux/fs/nfs/client.c
> >>
> >> So only the first file got included in the spatch invocation. But the
> >> spatch call gets fixed with all the files when quotes are added in the
> >> call to generate_deps_for_ns.
> >>
> >> But we need to include your change anyway, to make the script more
> >> robust.
> >
> >Hmm.
> >With this patch only, I see "bad variable name" error.
> >
> >
> >
> >masahiro@grover:~/ref/linux$ make -j8 nsdeps
> > DESCEND objtool
> > CALL scripts/atomic/check-atomics.sh
> > CALL scripts/checksyscalls.sh
> > CHK include/generated/compile.h
> > Building modules, stage 2.
> > MODPOST 20 modules
> >WARNING: module nfs uses symbol foo from namespace USB_STORAGE, but
> >does not import it.
> > Building modules, stage 2.
> > MODPOST 20 modules
> >./scripts/nsdeps: 34: local: ./fs/nfs/dir.c: bad variable name
> >make: *** [Makefile;1689: nsdeps] Error 2
>
> Hm, I was having trouble reproducing this until I changed the shell to
> dash, /bin/sh is a symlink to bash on my system, that might explain
> slightly different behavior. In any case, we should add quotes in both
> places.
>
> >> It would probably prevent more shell script related bugs in
> >> the future (Like [1]). I can respin this patch only while the other
> >> ones are superceded by your patchset.
> >>
> >> [1] https://unix.stackexchange.com/a/131767
> >
> >Anyway.
> >
> >Is this patch aiming for v5.4 (i.e. fixes) or v5.5-rc1 ?
>
> I am hoping for fixes, we should try to get all the small bugs out of
> nsdeps by 5.4 if we can..
>
> >If you touch the mod_source_files line,
> >we will have a conflict because
> >https://patchwork.kernel.org/patch/11217839/
> >is touching the same line.
> >
> >How should we organize the patch order?
>
> Would you like to fold these changes into your nsdeps improvements
> patchset? Since it's a pretty trivial change.
>
> Thanks!
>
>
My change set is quite big, so I
am planning to send it for v5.5-rc1.
If you want to fix nsdeps for composite modules earlier,
please apply this patch to your tree
and send another pull request.
--
Best Regards
Masahiro Yamada
+++ Masahiro Yamada [01/11/19 14:56 +0900]:
>On Thu, Oct 31, 2019 at 10:41 PM Jessica Yu <[email protected]> wrote:
>>
>> +++ Masahiro Yamada [31/10/19 21:27 +0900]:
>> >On Thu, Oct 31, 2019 at 1:17 AM Jessica Yu <[email protected]> wrote:
>> >>
>> >> +++ Masahiro Yamada [29/10/19 21:57 +0900]:
>> >> >On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
>> >> >>
>> >> >> The nsdeps script passes a list of the module source files to
>> >> >> generate_deps_for_ns() as a space delimited string named $mod_source_files,
>> >> >> which then passes it to spatch. But since $mod_source_files is not encased
>> >> >> in quotes, each source file in that string is treated as a separate shell
>> >> >> function argument (as $2, $3, $4, etc.). However, the spatch invocation
>> >> >> only refers to $2, so only the first file out of $mod_source_files is
>> >> >> processed by spatch.
>> >> >>
>> >> >> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
>> >> >> get inserted) when a module is composed of many source files and the
>> >> >> "main" module file containing the MODULE_LICENSE() statement is not the
>> >> >> first file listed in $mod_source_files. Fix this by encasing
>> >> >> $mod_source_files in quotes so that the entirety of the string is
>> >> >> treated as a single argument and can be referred to as $2.
>> >> >>
>> >> >> Signed-off-by: Jessica Yu <[email protected]>
>> >> >> ---
>> >> >> scripts/nsdeps | 2 +-
>> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >>
>> >> >> diff --git a/scripts/nsdeps b/scripts/nsdeps
>> >> >> index 9ddcd5cb96b1..5055b059a81b 100644
>> >> >> --- a/scripts/nsdeps
>> >> >> +++ b/scripts/nsdeps
>> >> >> @@ -36,7 +36,7 @@ generate_deps() {
>> >> >> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
>> >> >> for ns in `cat $ns_deps_file`; do
>> >> >> echo "Adding namespace $ns to module $mod_name (if needed)."
>> >> >> - generate_deps_for_ns $ns $mod_source_files
>> >> >> + generate_deps_for_ns $ns "$mod_source_files"
>> >> >> # sort the imports
>> >> >> for source_file in $mod_source_files; do
>> >> >> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
>> >> >
>> >> >I think this change is correct, but
>> >> >did you succeed in nsdeps for composite modules
>> >> >with this patch only?
>> >> >
>> >> >I think the following is needed too:
>> >> >
>> >> >
>> >> >diff --git a/scripts/nsdeps b/scripts/nsdeps
>> >> >index dda6fbac016e..5a23ea616446 100644
>> >> >--- a/scripts/nsdeps
>> >> >+++ b/scripts/nsdeps
>> >> >@@ -31,9 +31,9 @@ generate_deps() {
>> >> > local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
>> >> > local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
>> >> > if [ ! -f "$ns_deps_file" ]; then return; fi
>> >> >- local mod_source_files=`cat $mod_file | sed -n 1p \
>> >> >+ local mod_source_files="`cat $mod_file | sed -n 1p
>> >> > \
>> >> > | sed -e 's/\.o/\.c/g' \
>> >> >- | sed "s|[^ ]* *|${srctree}/&|g"`
>> >> >+ | sed "s|[^ ]* *|${srctree}/&|g"`"
>> >> > for ns in `cat $ns_deps_file`; do
>> >> > echo "Adding namespace $ns to module $mod_name (if needed)."
>> >> > generate_deps_for_ns $ns $mod_source_files
>> >> >
>> >> >
>> >> >Without this, a module that consists of two files
>> >> >will be expanded to:
>> >> >
>> >> >local mod_source_files=source1.c source2.c
>> >>
>> >> Yes, I was able to have nsdeps work for composite modules with just my
>> >> patch. Without this patch applied, the script produces the following
>> >> expansion of the generate_deps_for_ns call, (I just added a test
>> >> namespace MODULE):
>> >>
>> >> Adding namespace MODULE to module fs/nfs/nfs.ko.
>> >> + generate_deps_for_ns MODULE /tmp/ppyu/linux/fs/nfs/client.c /tmp/ppyu/linux/fs/nfs/dir.c /tmp/ppyu/linux/fs/nfs/file.c /tmp/ppyu/linux/fs/nfs/getroot.c /tmp/ppyu/linux/fs/nfs/inode.c /tmp/ppyu/linux/fs/nfs/super.c /tmp/ppyu/linux/fs/nfs/io.c /tmp/ppyu/linux/fs/nfs/direct.c /tmp/ppyu/linux/fs/nfs/pagelist.c /tmp/ppyu/linux/fs/nfs/read.c /tmp/ppyu/linux/fs/nfs/symlink.c /tmp/ppyu/linux/fs/nfs/unlink.c /tmp/ppyu/linux/fs/nfs/write.c /tmp/ppyu/linux/fs/nfs/namespace.c /tmp/ppyu/linux/fs/nfs/mount_clnt.c /tmp/ppyu/linux/fs/nfs/nfstrace.c /tmp/ppyu/linux/fs/nfs/export.c /tmp/ppyu/linux/fs/nfs/sysfs.c /tmp/ppyu/linux/fs/nfs/sysctl.c /tmp/ppyu/linux/fs/nfs/fscache.c /tmp/ppyu/linux/fs/nfs/fscache-index.c
>> >> + /usr/bin/spatch --very-quiet --in-place --sp-file /tmp/ppyu/linux/scripts/coccinelle/misc/add_namespace.cocci -D ns=MODULE /tmp/ppyu/linux/fs/nfs/client.c
>> >>
>> >> So only the first file got included in the spatch invocation. But the
>> >> spatch call gets fixed with all the files when quotes are added in the
>> >> call to generate_deps_for_ns.
>> >>
>> >> But we need to include your change anyway, to make the script more
>> >> robust.
>> >
>> >Hmm.
>> >With this patch only, I see "bad variable name" error.
>> >
>> >
>> >
>> >masahiro@grover:~/ref/linux$ make -j8 nsdeps
>> > DESCEND objtool
>> > CALL scripts/atomic/check-atomics.sh
>> > CALL scripts/checksyscalls.sh
>> > CHK include/generated/compile.h
>> > Building modules, stage 2.
>> > MODPOST 20 modules
>> >WARNING: module nfs uses symbol foo from namespace USB_STORAGE, but
>> >does not import it.
>> > Building modules, stage 2.
>> > MODPOST 20 modules
>> >./scripts/nsdeps: 34: local: ./fs/nfs/dir.c: bad variable name
>> >make: *** [Makefile;1689: nsdeps] Error 2
>>
>> Hm, I was having trouble reproducing this until I changed the shell to
>> dash, /bin/sh is a symlink to bash on my system, that might explain
>> slightly different behavior. In any case, we should add quotes in both
>> places.
>>
>> >> It would probably prevent more shell script related bugs in
>> >> the future (Like [1]). I can respin this patch only while the other
>> >> ones are superceded by your patchset.
>> >>
>> >> [1] https://unix.stackexchange.com/a/131767
>> >
>> >Anyway.
>> >
>> >Is this patch aiming for v5.4 (i.e. fixes) or v5.5-rc1 ?
>>
>> I am hoping for fixes, we should try to get all the small bugs out of
>> nsdeps by 5.4 if we can..
>>
>> >If you touch the mod_source_files line,
>> >we will have a conflict because
>> >https://patchwork.kernel.org/patch/11217839/
>> >is touching the same line.
>> >
>> >How should we organize the patch order?
>>
>> Would you like to fold these changes into your nsdeps improvements
>> patchset? Since it's a pretty trivial change.
>>
>> Thanks!
>>
>>
>
>My change set is quite big, so I
>am planning to send it for v5.5-rc1.
OK, sounds good.
>If you want to fix nsdeps for composite modules earlier,
>please apply this patch to your tree
>and send another pull request.
I plan to send this in for -rc7, since it's a minor fix.
For the conflict, if these patches are not pushed anywhere yet (at
least I did not see them on the kbuild for-next branch yet), perhaps
you could rebase on top of -rc7 and put your patchset on top?
Thanks,
Jessica
On Tue, Nov 5, 2019 at 9:52 PM Jessica Yu <[email protected]> wrote:
>
> +++ Masahiro Yamada [01/11/19 14:56 +0900]:
> >On Thu, Oct 31, 2019 at 10:41 PM Jessica Yu <[email protected]> wrote:
> >>
> >> +++ Masahiro Yamada [31/10/19 21:27 +0900]:
> >> >On Thu, Oct 31, 2019 at 1:17 AM Jessica Yu <[email protected]> wrote:
> >> >>
> >> >> +++ Masahiro Yamada [29/10/19 21:57 +0900]:
> >> >> >On Tue, Oct 29, 2019 at 12:14 AM Jessica Yu <[email protected]> wrote:
> >> >> >>
> >> >> >> The nsdeps script passes a list of the module source files to
> >> >> >> generate_deps_for_ns() as a space delimited string named $mod_source_files,
> >> >> >> which then passes it to spatch. But since $mod_source_files is not encased
> >> >> >> in quotes, each source file in that string is treated as a separate shell
> >> >> >> function argument (as $2, $3, $4, etc.). However, the spatch invocation
> >> >> >> only refers to $2, so only the first file out of $mod_source_files is
> >> >> >> processed by spatch.
> >> >> >>
> >> >> >> This causes problems (namely, the MODULE_IMPORT_NS() statement doesn't
> >> >> >> get inserted) when a module is composed of many source files and the
> >> >> >> "main" module file containing the MODULE_LICENSE() statement is not the
> >> >> >> first file listed in $mod_source_files. Fix this by encasing
> >> >> >> $mod_source_files in quotes so that the entirety of the string is
> >> >> >> treated as a single argument and can be referred to as $2.
> >> >> >>
> >> >> >> Signed-off-by: Jessica Yu <[email protected]>
> >> >> >> ---
> >> >> >> scripts/nsdeps | 2 +-
> >> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >>
> >> >> >> diff --git a/scripts/nsdeps b/scripts/nsdeps
> >> >> >> index 9ddcd5cb96b1..5055b059a81b 100644
> >> >> >> --- a/scripts/nsdeps
> >> >> >> +++ b/scripts/nsdeps
> >> >> >> @@ -36,7 +36,7 @@ generate_deps() {
> >> >> >> | sed -E "s%(^|\s)([^/][^ ]*)%\1$srctree/\2%g"`
> >> >> >> for ns in `cat $ns_deps_file`; do
> >> >> >> echo "Adding namespace $ns to module $mod_name (if needed)."
> >> >> >> - generate_deps_for_ns $ns $mod_source_files
> >> >> >> + generate_deps_for_ns $ns "$mod_source_files"
> >> >> >> # sort the imports
> >> >> >> for source_file in $mod_source_files; do
> >> >> >> sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp
> >> >> >
> >> >> >I think this change is correct, but
> >> >> >did you succeed in nsdeps for composite modules
> >> >> >with this patch only?
> >> >> >
> >> >> >I think the following is needed too:
> >> >> >
> >> >> >
> >> >> >diff --git a/scripts/nsdeps b/scripts/nsdeps
> >> >> >index dda6fbac016e..5a23ea616446 100644
> >> >> >--- a/scripts/nsdeps
> >> >> >+++ b/scripts/nsdeps
> >> >> >@@ -31,9 +31,9 @@ generate_deps() {
> >> >> > local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'`
> >> >> > local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'`
> >> >> > if [ ! -f "$ns_deps_file" ]; then return; fi
> >> >> >- local mod_source_files=`cat $mod_file | sed -n 1p \
> >> >> >+ local mod_source_files="`cat $mod_file | sed -n 1p
> >> >> > \
> >> >> > | sed -e 's/\.o/\.c/g' \
> >> >> >- | sed "s|[^ ]* *|${srctree}/&|g"`
> >> >> >+ | sed "s|[^ ]* *|${srctree}/&|g"`"
> >> >> > for ns in `cat $ns_deps_file`; do
> >> >> > echo "Adding namespace $ns to module $mod_name (if needed)."
> >> >> > generate_deps_for_ns $ns $mod_source_files
> >> >> >
> >> >> >
> >> >> >Without this, a module that consists of two files
> >> >> >will be expanded to:
> >> >> >
> >> >> >local mod_source_files=source1.c source2.c
> >> >>
> >> >> Yes, I was able to have nsdeps work for composite modules with just my
> >> >> patch. Without this patch applied, the script produces the following
> >> >> expansion of the generate_deps_for_ns call, (I just added a test
> >> >> namespace MODULE):
> >> >>
> >> >> Adding namespace MODULE to module fs/nfs/nfs.ko.
> >> >> + generate_deps_for_ns MODULE /tmp/ppyu/linux/fs/nfs/client.c /tmp/ppyu/linux/fs/nfs/dir.c /tmp/ppyu/linux/fs/nfs/file.c /tmp/ppyu/linux/fs/nfs/getroot.c /tmp/ppyu/linux/fs/nfs/inode.c /tmp/ppyu/linux/fs/nfs/super.c /tmp/ppyu/linux/fs/nfs/io.c /tmp/ppyu/linux/fs/nfs/direct.c /tmp/ppyu/linux/fs/nfs/pagelist.c /tmp/ppyu/linux/fs/nfs/read.c /tmp/ppyu/linux/fs/nfs/symlink.c /tmp/ppyu/linux/fs/nfs/unlink.c /tmp/ppyu/linux/fs/nfs/write.c /tmp/ppyu/linux/fs/nfs/namespace.c /tmp/ppyu/linux/fs/nfs/mount_clnt.c /tmp/ppyu/linux/fs/nfs/nfstrace.c /tmp/ppyu/linux/fs/nfs/export.c /tmp/ppyu/linux/fs/nfs/sysfs.c /tmp/ppyu/linux/fs/nfs/sysctl.c /tmp/ppyu/linux/fs/nfs/fscache.c /tmp/ppyu/linux/fs/nfs/fscache-index.c
> >> >> + /usr/bin/spatch --very-quiet --in-place --sp-file /tmp/ppyu/linux/scripts/coccinelle/misc/add_namespace.cocci -D ns=MODULE /tmp/ppyu/linux/fs/nfs/client.c
> >> >>
> >> >> So only the first file got included in the spatch invocation. But the
> >> >> spatch call gets fixed with all the files when quotes are added in the
> >> >> call to generate_deps_for_ns.
> >> >>
> >> >> But we need to include your change anyway, to make the script more
> >> >> robust.
> >> >
> >> >Hmm.
> >> >With this patch only, I see "bad variable name" error.
> >> >
> >> >
> >> >
> >> >masahiro@grover:~/ref/linux$ make -j8 nsdeps
> >> > DESCEND objtool
> >> > CALL scripts/atomic/check-atomics.sh
> >> > CALL scripts/checksyscalls.sh
> >> > CHK include/generated/compile.h
> >> > Building modules, stage 2.
> >> > MODPOST 20 modules
> >> >WARNING: module nfs uses symbol foo from namespace USB_STORAGE, but
> >> >does not import it.
> >> > Building modules, stage 2.
> >> > MODPOST 20 modules
> >> >./scripts/nsdeps: 34: local: ./fs/nfs/dir.c: bad variable name
> >> >make: *** [Makefile;1689: nsdeps] Error 2
> >>
> >> Hm, I was having trouble reproducing this until I changed the shell to
> >> dash, /bin/sh is a symlink to bash on my system, that might explain
> >> slightly different behavior. In any case, we should add quotes in both
> >> places.
> >>
> >> >> It would probably prevent more shell script related bugs in
> >> >> the future (Like [1]). I can respin this patch only while the other
> >> >> ones are superceded by your patchset.
> >> >>
> >> >> [1] https://unix.stackexchange.com/a/131767
> >> >
> >> >Anyway.
> >> >
> >> >Is this patch aiming for v5.4 (i.e. fixes) or v5.5-rc1 ?
> >>
> >> I am hoping for fixes, we should try to get all the small bugs out of
> >> nsdeps by 5.4 if we can..
> >>
> >> >If you touch the mod_source_files line,
> >> >we will have a conflict because
> >> >https://patchwork.kernel.org/patch/11217839/
> >> >is touching the same line.
> >> >
> >> >How should we organize the patch order?
> >>
> >> Would you like to fold these changes into your nsdeps improvements
> >> patchset? Since it's a pretty trivial change.
> >>
> >> Thanks!
> >>
> >>
> >
> >My change set is quite big, so I
> >am planning to send it for v5.5-rc1.
>
> OK, sounds good.
>
> >If you want to fix nsdeps for composite modules earlier,
> >please apply this patch to your tree
> >and send another pull request.
>
> I plan to send this in for -rc7, since it's a minor fix.
OK.
> For the conflict, if these patches are not pushed anywhere yet (at
> least I did not see them on the kbuild for-next branch yet), perhaps
> you could rebase on top of -rc7 and put your patchset on top?
No problem. I will do so.
Thanks.
--
Best Regards
Masahiro Yamada