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 <[email protected]>
---
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
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 <[email protected]>
---
This is an alternative to my first patch here:
http://lore.kernel.org/r/[email protected]
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
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 <[email protected]>
> ---
> 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)
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 <[email protected]>
>---
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 <[email protected]>
Tested-by: Matthias Maennich <[email protected]>
Cheers,
Matthias
>
>This is an alternative to my first patch here:
>
> http://lore.kernel.org/r/[email protected]
>
>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
>
On Tue, Oct 22, 2019 at 1:05 AM Jessica Yu <[email protected]> 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 <[email protected]>
> ---
>
> This is an alternative to my first patch here:
>
> http://lore.kernel.org/r/[email protected]
>
> 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
+++ Masahiro Yamada [22/10/19 13:37 +0900]:
>On Tue, Oct 22, 2019 at 1:05 AM Jessica Yu <[email protected]> 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 <[email protected]>
>> ---
>>
>> This is an alternative to my first patch here:
>>
>> http://lore.kernel.org/r/[email protected]
>>
>> 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!
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 <[email protected]>
---
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
+++ 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 <[email protected]>
>> ---
>> 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.
On Tue, Oct 22, 2019 at 8:04 PM Jessica Yu <[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
--
Best Regards
Masahiro Yamada
On Wed, Oct 23, 2019 at 10:23:39AM +0900, Masahiro Yamada wrote:
>On Tue, Oct 22, 2019 at 8:04 PM Jessica Yu <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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 <[email protected]>
>
Tested-by: Matthias Maennich <[email protected]>
Cheers,
Matthias
+++ 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 <[email protected]> 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 <[email protected]>
>>>---
>>>
>>>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 <[email protected]>
>>
>
>Tested-by: Matthias Maennich <[email protected]>
Applied, thanks!
Jessica