[PATCH] scripts/nsdeps: escape '/' for module source files

[PATCH] scripts/nsdeps: escape '/' for module source files

[Jessica Yu]

When doing an out of tree build with O=, the nsdeps script constructs
the absolute pathname of the module source file so that it can insert
MODULE_IMPORT_NS statements in the right place. However, ${srctree}
contains an unescaped path to the source tree, which, when used in a sed
substitution, makes sed complain:

++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
sed: -e expression #1, char 12: unknown option to `s'

The sed substitution command 's' ends prematurely with the forward
slashes in the pathname, and sed errors out when it encounters the 'h',
which is an invalid sed substitution option. So use bash in-variable
substitution to escape all forward slashes for sed.

Signed-off-by: Jessica Yu <jeyu@kernel.org>
---
 scripts/nsdeps | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/nsdeps b/scripts/nsdeps
index 3754dac13b31..79f96e596a0b 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -33,7 +33,7 @@ generate_deps() {
 	if [ ! -f "$ns_deps_file" ]; then return; fi
 	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
-- 
2.16.4

[PATCH v2] scripts/nsdeps: use alternative sed delimiter

[Jessica Yu]

When doing an out of tree build with O=, the nsdeps script constructs
the absolute pathname of the module source file so that it can insert
MODULE_IMPORT_NS statements in the right place. However, ${srctree}
contains an unescaped path to the source tree, which, when used in a sed
substitution, makes sed complain:

++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
sed: -e expression #1, char 12: unknown option to `s'

The sed substitution command 's' ends prematurely with the forward
slashes in the pathname, and sed errors out when it encounters the 'h',
which is an invalid sed substitution option. To avoid escaping forward
slashes in ${srctree}, we can use '|' as an alternative delimiter for
sed to avoid this error.

Signed-off-by: Jessica Yu <jeyu@kernel.org>
---

This is an alternative to my first patch here:

  http://lore.kernel.org/r/20191021145137.31672-1-jeyu@kernel.org

Matthias suggested using an alternative sed delimiter instead to avoid the
ugly/unreadable ${srctree//\//\\\/} substitution.

 scripts/nsdeps | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/nsdeps b/scripts/nsdeps
index 3754dac13b31..63da30a33422 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -33,7 +33,7 @@ generate_deps() {
 	if [ ! -f "$ns_deps_file" ]; then return; fi
 	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
-- 
2.16.4

Re: [PATCH v2] scripts/nsdeps: use alternative sed delimiter

[Matthias Maennich]

Hi Jessica!

On Mon, Oct 21, 2019 at 06:04:19PM +0200, Jessica Yu wrote:
>When doing an out of tree build with O=, the nsdeps script constructs
>the absolute pathname of the module source file so that it can insert
>MODULE_IMPORT_NS statements in the right place. However, ${srctree}
>contains an unescaped path to the source tree, which, when used in a sed
>substitution, makes sed complain:
>
>++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
>sed: -e expression #1, char 12: unknown option to `s'
>
>The sed substitution command 's' ends prematurely with the forward
>slashes in the pathname, and sed errors out when it encounters the 'h',
>which is an invalid sed substitution option. To avoid escaping forward
>slashes in ${srctree}, we can use '|' as an alternative delimiter for
>sed to avoid this error.
>
>Signed-off-by: Jessica Yu <jeyu@kernel.org>
>---

Thanks for fixing this. I tested O=, but not with a truly out of tree
build and got outsmarted by ${srctree} being '..' for O=subdir/.

Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>
>This is an alternative to my first patch here:
>
>  http://lore.kernel.org/r/20191021145137.31672-1-jeyu@kernel.org
>
>Matthias suggested using an alternative sed delimiter instead to avoid the
>ugly/unreadable ${srctree//\//\\\/} substitution.
>
> scripts/nsdeps | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/scripts/nsdeps b/scripts/nsdeps
>index 3754dac13b31..63da30a33422 100644
>--- a/scripts/nsdeps
>+++ b/scripts/nsdeps
>@@ -33,7 +33,7 @@ generate_deps() {
> 	if [ ! -f "$ns_deps_file" ]; then return; fi
> 	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
>-- 
>2.16.4
>

Re: [PATCH v2] scripts/nsdeps: use alternative sed delimiter

[Masahiro Yamada]

On Tue, Oct 22, 2019 at 1:05 AM Jessica Yu <jeyu@kernel.org> wrote:
>
> When doing an out of tree build with O=, the nsdeps script constructs
> the absolute pathname of the module source file so that it can insert
> MODULE_IMPORT_NS statements in the right place. However, ${srctree}
> contains an unescaped path to the source tree, which, when used in a sed
> substitution, makes sed complain:
>
> ++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
> sed: -e expression #1, char 12: unknown option to `s'
>
> The sed substitution command 's' ends prematurely with the forward
> slashes in the pathname, and sed errors out when it encounters the 'h',
> which is an invalid sed substitution option. To avoid escaping forward
> slashes in ${srctree}, we can use '|' as an alternative delimiter for
> sed to avoid this error.
>
> Signed-off-by: Jessica Yu <jeyu@kernel.org>
> ---
>
> This is an alternative to my first patch here:
>
>   http://lore.kernel.org/r/20191021145137.31672-1-jeyu@kernel.org
>
> Matthias suggested using an alternative sed delimiter instead to avoid the
> ugly/unreadable ${srctree//\//\\\/} substitution.
>
>  scripts/nsdeps | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index 3754dac13b31..63da30a33422 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -33,7 +33,7 @@ generate_deps() {
>         if [ ! -f "$ns_deps_file" ]; then return; fi
>         local mod_source_files=`cat $mod_file | sed -n 1p                      \
>                                               | sed -e 's/\.o/\.c/g'           \
> -                                             | sed "s/[^ ]* */${srctree}\/&/g"`
> +                                             | sed "s|[^ ]* *|${srctree}\/&|g"`


You no longer need to escape the '/'.

s|[^ ]* *|${srctree}/&|g

is enough.


BTW, connecting multiple sed commands with pipes
is not efficient.


sed -n 1p | sed -e 's/\.o/\.c/g'

can be replaced with

sed -n '1s/\.o/\.c/gp'



This script can be improved a whole
if somebody is interested in the refactoring.




>         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
> --
> 2.16.4
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] scripts/nsdeps: use alternative sed delimiter

[Jessica Yu]

+++ Masahiro Yamada [22/10/19 13:37 +0900]:
>On Tue, Oct 22, 2019 at 1:05 AM Jessica Yu <jeyu@kernel.org> wrote:
>>
>> When doing an out of tree build with O=, the nsdeps script constructs
>> the absolute pathname of the module source file so that it can insert
>> MODULE_IMPORT_NS statements in the right place. However, ${srctree}
>> contains an unescaped path to the source tree, which, when used in a sed
>> substitution, makes sed complain:
>>
>> ++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
>> sed: -e expression #1, char 12: unknown option to `s'
>>
>> The sed substitution command 's' ends prematurely with the forward
>> slashes in the pathname, and sed errors out when it encounters the 'h',
>> which is an invalid sed substitution option. To avoid escaping forward
>> slashes in ${srctree}, we can use '|' as an alternative delimiter for
>> sed to avoid this error.
>>
>> Signed-off-by: Jessica Yu <jeyu@kernel.org>
>> ---
>>
>> This is an alternative to my first patch here:
>>
>>   http://lore.kernel.org/r/20191021145137.31672-1-jeyu@kernel.org
>>
>> Matthias suggested using an alternative sed delimiter instead to avoid the
>> ugly/unreadable ${srctree//\//\\\/} substitution.
>>
>>  scripts/nsdeps | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/nsdeps b/scripts/nsdeps
>> index 3754dac13b31..63da30a33422 100644
>> --- a/scripts/nsdeps
>> +++ b/scripts/nsdeps
>> @@ -33,7 +33,7 @@ generate_deps() {
>>         if [ ! -f "$ns_deps_file" ]; then return; fi
>>         local mod_source_files=`cat $mod_file | sed -n 1p                      \
>>                                               | sed -e 's/\.o/\.c/g'           \
>> -                                             | sed "s/[^ ]* */${srctree}\/&/g"`
>> +                                             | sed "s|[^ ]* *|${srctree}\/&|g"`
>
>
>You no longer need to escape the '/'.
>
>s|[^ ]* *|${srctree}/&|g
>
>is enough.

Ah yeah, I missed that. Thanks for catching that!

[PATCH v3] scripts/nsdeps: use alternative sed delimiter

[Jessica Yu]

When doing an out of tree build with O=, the nsdeps script constructs
the absolute pathname of the module source file so that it can insert
MODULE_IMPORT_NS statements in the right place. However, ${srctree}
contains an unescaped path to the source tree, which, when used in a sed
substitution, makes sed complain:

++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
sed: -e expression #1, char 12: unknown option to `s'

The sed substitution command 's' ends prematurely with the forward
slashes in the pathname, and sed errors out when it encounters the 'h',
which is an invalid sed substitution option. To avoid escaping forward
slashes ${srctree}, we can use '|' as an alternative delimiter for
sed instead to avoid this error.

Signed-off-by: Jessica Yu <jeyu@kernel.org>
---

v3: don't need to escape '/' since we're using a different delimiter.

 scripts/nsdeps | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/nsdeps b/scripts/nsdeps
index 3754dac13b31..dda6fbac016e 100644
--- a/scripts/nsdeps
+++ b/scripts/nsdeps
@@ -33,7 +33,7 @@ generate_deps() {
 	if [ ! -f "$ns_deps_file" ]; then return; fi
 	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
-- 
2.16.4

Re: [PATCH v3] scripts/nsdeps: use alternative sed delimiter

[Masahiro Yamada]

On Tue, Oct 22, 2019 at 8:04 PM Jessica Yu <jeyu@kernel.org> wrote:
>
> When doing an out of tree build with O=, the nsdeps script constructs
> the absolute pathname of the module source file so that it can insert
> MODULE_IMPORT_NS statements in the right place. However, ${srctree}
> contains an unescaped path to the source tree, which, when used in a sed
> substitution, makes sed complain:
>
> ++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
> sed: -e expression #1, char 12: unknown option to `s'
>
> The sed substitution command 's' ends prematurely with the forward
> slashes in the pathname, and sed errors out when it encounters the 'h',
> which is an invalid sed substitution option. To avoid escaping forward
> slashes ${srctree}, we can use '|' as an alternative delimiter for
> sed instead to avoid this error.
>
> Signed-off-by: Jessica Yu <jeyu@kernel.org>
> ---
>
> v3: don't need to escape '/' since we're using a different delimiter.
>
>  scripts/nsdeps | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index 3754dac13b31..dda6fbac016e 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -33,7 +33,7 @@ generate_deps() {
>         if [ ! -f "$ns_deps_file" ]; then return; fi
>         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
> --
> 2.16.4
>

Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>

-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3] scripts/nsdeps: use alternative sed delimiter

[Matthias Maennich]

On Wed, Oct 23, 2019 at 10:23:39AM +0900, Masahiro Yamada wrote:
>On Tue, Oct 22, 2019 at 8:04 PM Jessica Yu <jeyu@kernel.org> wrote:
>>
>> When doing an out of tree build with O=, the nsdeps script constructs
>> the absolute pathname of the module source file so that it can insert
>> MODULE_IMPORT_NS statements in the right place. However, ${srctree}
>> contains an unescaped path to the source tree, which, when used in a sed
>> substitution, makes sed complain:
>>
>> ++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
>> sed: -e expression #1, char 12: unknown option to `s'
>>
>> The sed substitution command 's' ends prematurely with the forward
>> slashes in the pathname, and sed errors out when it encounters the 'h',
>> which is an invalid sed substitution option. To avoid escaping forward
>> slashes ${srctree}, we can use '|' as an alternative delimiter for
>> sed instead to avoid this error.
>>
>> Signed-off-by: Jessica Yu <jeyu@kernel.org>
>> ---
>>
>> v3: don't need to escape '/' since we're using a different delimiter.
>>
>>  scripts/nsdeps | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/nsdeps b/scripts/nsdeps
>> index 3754dac13b31..dda6fbac016e 100644
>> --- a/scripts/nsdeps
>> +++ b/scripts/nsdeps
>> @@ -33,7 +33,7 @@ generate_deps() {
>>         if [ ! -f "$ns_deps_file" ]; then return; fi
>>         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
>> --
>> 2.16.4
>>
>
>Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>

Tested-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

Re: [PATCH v3] scripts/nsdeps: use alternative sed delimiter

[Jessica Yu]

+++ Matthias Maennich [23/10/19 11:13 +0100]:
>On Wed, Oct 23, 2019 at 10:23:39AM +0900, Masahiro Yamada wrote:
>>On Tue, Oct 22, 2019 at 8:04 PM Jessica Yu <jeyu@kernel.org> wrote:
>>>
>>>When doing an out of tree build with O=, the nsdeps script constructs
>>>the absolute pathname of the module source file so that it can insert
>>>MODULE_IMPORT_NS statements in the right place. However, ${srctree}
>>>contains an unescaped path to the source tree, which, when used in a sed
>>>substitution, makes sed complain:
>>>
>>>++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
>>>sed: -e expression #1, char 12: unknown option to `s'
>>>
>>>The sed substitution command 's' ends prematurely with the forward
>>>slashes in the pathname, and sed errors out when it encounters the 'h',
>>>which is an invalid sed substitution option. To avoid escaping forward
>>>slashes ${srctree}, we can use '|' as an alternative delimiter for
>>>sed instead to avoid this error.
>>>
>>>Signed-off-by: Jessica Yu <jeyu@kernel.org>
>>>---
>>>
>>>v3: don't need to escape '/' since we're using a different delimiter.
>>>
>>> scripts/nsdeps | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/scripts/nsdeps b/scripts/nsdeps
>>>index 3754dac13b31..dda6fbac016e 100644
>>>--- a/scripts/nsdeps
>>>+++ b/scripts/nsdeps
>>>@@ -33,7 +33,7 @@ generate_deps() {
>>>        if [ ! -f "$ns_deps_file" ]; then return; fi
>>>        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
>>>--
>>>2.16.4
>>>
>>
>>Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>
>
>Tested-by: Matthias Maennich <maennich@google.com>

Applied, thanks!

Jessica

RE: [PATCH] scripts/nsdeps: escape '/' for module source files

[David Laight]

From Jessica Yu
> Sent: 21 October 2019 15:52
> When doing an out of tree build with O=, the nsdeps script constructs
> the absolute pathname of the module source file so that it can insert
> MODULE_IMPORT_NS statements in the right place. However, ${srctree}
> contains an unescaped path to the source tree, which, when used in a sed
> substitution, makes sed complain:
> 
> ++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
> sed: -e expression #1, char 12: unknown option to `s'
> 
> The sed substitution command 's' ends prematurely with the forward
> slashes in the pathname, and sed errors out when it encounters the 'h',
> which is an invalid sed substitution option. So use bash in-variable
> substitution to escape all forward slashes for sed.
> 
> Signed-off-by: Jessica Yu <jeyu@kernel.org>
> ---
>  scripts/nsdeps | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/nsdeps b/scripts/nsdeps
> index 3754dac13b31..79f96e596a0b 100644
> --- a/scripts/nsdeps
> +++ b/scripts/nsdeps
> @@ -33,7 +33,7 @@ generate_deps() {
>  	if [ ! -f "$ns_deps_file" ]; then return; fi
>  	local mod_source_files=`cat $mod_file | sed -n 1p                      \
>  					      | sed -e 's/\.o/\.c/g'           \
> -					      | sed "s/[^ ]* */${srctree}\/&/g"`
> +					      | sed "s/[^ ]* */${srctree//\//\\\/}\/&/g"`

Rather than adding a bashism - which might bight back later, just change the
command to use (say) ; instead of / as the separator.
I think that makes it:
	sed "s;[^ ]* *;${srctree}/&;g

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] scripts/nsdeps: escape '/' for module source files

[Jessica Yu]

+++ David Laight [21/10/19 16:14 +0000]:
>From Jessica Yu
>> Sent: 21 October 2019 15:52
>> When doing an out of tree build with O=, the nsdeps script constructs
>> the absolute pathname of the module source file so that it can insert
>> MODULE_IMPORT_NS statements in the right place. However, ${srctree}
>> contains an unescaped path to the source tree, which, when used in a sed
>> substitution, makes sed complain:
>>
>> ++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
>> sed: -e expression #1, char 12: unknown option to `s'
>>
>> The sed substitution command 's' ends prematurely with the forward
>> slashes in the pathname, and sed errors out when it encounters the 'h',
>> which is an invalid sed substitution option. So use bash in-variable
>> substitution to escape all forward slashes for sed.
>>
>> Signed-off-by: Jessica Yu <jeyu@kernel.org>
>> ---
>>  scripts/nsdeps | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/nsdeps b/scripts/nsdeps
>> index 3754dac13b31..79f96e596a0b 100644
>> --- a/scripts/nsdeps
>> +++ b/scripts/nsdeps
>> @@ -33,7 +33,7 @@ generate_deps() {
>>  	if [ ! -f "$ns_deps_file" ]; then return; fi
>>  	local mod_source_files=`cat $mod_file | sed -n 1p                      \
>>  					      | sed -e 's/\.o/\.c/g'           \
>> -					      | sed "s/[^ ]* */${srctree}\/&/g"`
>> +					      | sed "s/[^ ]* */${srctree//\//\\\/}\/&/g"`
>
>Rather than adding a bashism - which might bight back later, just change the
>command to use (say) ; instead of / as the separator.
>I think that makes it:
>	sed "s;[^ ]* *;${srctree}/&;g
>
>	David

Thanks David! Matthias suggested this as well and so I've sent a v3.