* Re: [PATCH v2] scripts/nsdeps: use alternative sed delimiter
2019-10-21 16:04 ` [PATCH v2] scripts/nsdeps: use alternative sed delimiter Jessica Yu
@ 2019-10-21 16:16 ` Matthias Maennich
2019-10-22 4:37 ` Masahiro Yamada
2019-10-22 11:04 ` [PATCH v3] " Jessica Yu
2 siblings, 0 replies; 11+ messages in thread
From: Matthias Maennich @ 2019-10-21 16:16 UTC (permalink / raw)
To: Jessica Yu; +Cc: linux-kernel, Masahiro Yamada
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] scripts/nsdeps: use alternative sed delimiter
2019-10-21 16:04 ` [PATCH v2] scripts/nsdeps: use alternative sed delimiter Jessica Yu
2019-10-21 16:16 ` Matthias Maennich
@ 2019-10-22 4:37 ` Masahiro Yamada
2019-10-22 10:00 ` Jessica Yu
2019-10-22 11:04 ` [PATCH v3] " Jessica Yu
2 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2019-10-22 4:37 UTC (permalink / raw)
To: Jessica Yu; +Cc: Linux Kernel Mailing List, Matthias Maennich
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] scripts/nsdeps: use alternative sed delimiter
2019-10-22 4:37 ` Masahiro Yamada
@ 2019-10-22 10:00 ` Jessica Yu
0 siblings, 0 replies; 11+ messages in thread
From: Jessica Yu @ 2019-10-22 10:00 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Linux Kernel Mailing List, Matthias Maennich
+++ 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!
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] scripts/nsdeps: use alternative sed delimiter
2019-10-21 16:04 ` [PATCH v2] scripts/nsdeps: use alternative sed delimiter Jessica Yu
2019-10-21 16:16 ` Matthias Maennich
2019-10-22 4:37 ` Masahiro Yamada
@ 2019-10-22 11:04 ` Jessica Yu
2019-10-23 1:23 ` Masahiro Yamada
2 siblings, 1 reply; 11+ messages in thread
From: Jessica Yu @ 2019-10-22 11:04 UTC (permalink / raw)
To: linux-kernel; +Cc: Matthias Maennich, Masahiro Yamada, David Laight, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3] scripts/nsdeps: use alternative sed delimiter
2019-10-22 11:04 ` [PATCH v3] " Jessica Yu
@ 2019-10-23 1:23 ` Masahiro Yamada
2019-10-23 10:13 ` Matthias Maennich
0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2019-10-23 1:23 UTC (permalink / raw)
To: Jessica Yu; +Cc: Linux Kernel Mailing List, Matthias Maennich, David Laight
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] scripts/nsdeps: use alternative sed delimiter
2019-10-23 1:23 ` Masahiro Yamada
@ 2019-10-23 10:13 ` Matthias Maennich
2019-10-23 10:15 ` Jessica Yu
0 siblings, 1 reply; 11+ messages in thread
From: Matthias Maennich @ 2019-10-23 10:13 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Jessica Yu, Linux Kernel Mailing List, David Laight
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] scripts/nsdeps: use alternative sed delimiter
2019-10-23 10:13 ` Matthias Maennich
@ 2019-10-23 10:15 ` Jessica Yu
0 siblings, 0 replies; 11+ messages in thread
From: Jessica Yu @ 2019-10-23 10:15 UTC (permalink / raw)
To: Matthias Maennich
Cc: Masahiro Yamada, Linux Kernel Mailing List, David Laight
+++ 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
^ permalink raw reply [flat|nested] 11+ messages in thread