[PATCH] build: fix sed usage in build process

[PATCH] build: fix sed usage in build process

[Roger Pau Monne]

FreeBSD sed is not able to correctly parse the script
'/[0-9]/{s,00*,0,g;p}', so break it into two smaller scripts which
FreeBSD (and Linux) sed is able to parse.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
---
 xen/Rules.mk                 |    2 +-
 xen/arch/x86/boot/build32.mk |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3a6cec5..9d5223b 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -171,7 +171,7 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach n,1 2 4 8,rodata.str1.$(n)) \
 			 $(foreach r,rel rel.ro,data.$(r) data.$(r).local)
 
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
-	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p}' | while read idx name sz rest; do \
+	$(OBJDUMP) -h $< | sed -re 's/0+/0/g' -ne '/[0-9]/p' | while read idx name sz rest; do \
 		case "$$name" in \
 		.text|.text.*|.data|.data.*|.bss) \
 			test $$sz != 0 || continue; \
diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot/build32.mk
index a2d4b34..c5a8890 100644
--- a/xen/arch/x86/boot/build32.mk
+++ b/xen/arch/x86/boot/build32.mk
@@ -20,7 +20,7 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
 
 %.o: %.c
 	$(CC) $(CFLAGS) -c -fpic $< -o $@
-	$(OBJDUMP) -h $@ | sed -n '/[0-9]/{s,00*,0,g;p}' |\
+	$(OBJDUMP) -h $@ | sed -re 's/0+/0/g' -ne '/[0-9]/p' |\
 		while read idx name sz rest; do \
 			case "$$name" in \
 			.data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \
-- 
1.7.7.5 (Apple Git-26)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] build: fix sed usage in build process

[Jan Beulich]

>>> On 25.03.14 at 12:20, <roger.pau@citrix.com> wrote:
> FreeBSD sed is not able to correctly parse the script
> '/[0-9]/{s,00*,0,g;p}', so break it into two smaller scripts which
> FreeBSD (and Linux) sed is able to parse.

First of all - is this again a standard conformance issue? I can't see
what's non-conformant with the old approach (which btw had been
in place for quite long a time, so I'm puzzled by this being an issue
only now), and for future reference purposes it would be nice to
know what exactly should be avoided (i.e. to prevent a similar issue
from getting introduced again later).

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -171,7 +171,7 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach n,1 2 4 
> 8,rodata.str1.$(n)) \
>  			 $(foreach r,rel rel.ro,data.$(r) data.$(r).local)
>  
>  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
> -	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p}' | while read idx name sz rest; do \
> +	$(OBJDUMP) -h $< | sed -re 's/0+/0/g' -ne '/[0-9]/p' | while read idx name sz rest; do \

Is there a particular reason for moving the -n to the second expression
specification?

And I take it that you adding -r and the use of + isn't really necessary
either - the 00* approach should work equally well without using the
non-standard (but apparently more wide spread) -r? (Yes, we're just
about to commit other uses of -r, but there the alternative would be
more difficult to grok, so has better justification.)

Jan

Re: [PATCH] build: fix sed usage in build process

[Roger Pau Monné]

On 25/03/14 15:06, Jan Beulich wrote:
>>>> On 25.03.14 at 12:20, <roger.pau@citrix.com> wrote:
>> FreeBSD sed is not able to correctly parse the script
>> '/[0-9]/{s,00*,0,g;p}', so break it into two smaller scripts which
>> FreeBSD (and Linux) sed is able to parse.
> 
> First of all - is this again a standard conformance issue? I can't see
> what's non-conformant with the old approach (which btw had been
> in place for quite long a time, so I'm puzzled by this being an issue
> only now), and for future reference purposes it would be nice to
> know what exactly should be avoided (i.e. to prevent a similar issue
> from getting introduced again later).

Sorry, my regex skills are quite basic. The error is the following:

sed: 1: "/[0-9]/{s,00*,0,g;p}": extra characters at the end of p command

I just never noticed such errors, since the build finishes successfully,
and the binary boots without problems.

> 
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -171,7 +171,7 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach n,1 2 4 
>> 8,rodata.str1.$(n)) \
>>  			 $(foreach r,rel rel.ro,data.$(r) data.$(r).local)
>>  
>>  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
>> -	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p}' | while read idx name sz rest; do \
>> +	$(OBJDUMP) -h $< | sed -re 's/0+/0/g' -ne '/[0-9]/p' | while read idx name sz rest; do \
> 
> Is there a particular reason for moving the -n to the second expression
> specification?

No, moved it back to the first one.

> 
> And I take it that you adding -r and the use of + isn't really necessary
> either - the 00* approach should work equally well without using the
> non-standard (but apparently more wide spread) -r? (Yes, we're just
> about to commit other uses of -r, but there the alternative would be
> more difficult to grok, so has better justification.)

I just thought the use of 0+ was clearer than 00*, but if you prefer to
avoid using -r in more places the following expression also works on
FreeBSD and is more similar to the original one:

sed -ne 's/00*/0/g' -e '/[0-9]/p'

If you think this is a suitable solution I can resend the patch (or if
it's OK you can modify it before committing).

Roger.

Re: [PATCH] build: fix sed usage in build process

[Jan Beulich]

>>> On 25.03.14 at 16:58, <roger.pau@citrix.com> wrote:
> On 25/03/14 15:06, Jan Beulich wrote:
>>>>> On 25.03.14 at 12:20, <roger.pau@citrix.com> wrote:
>>> FreeBSD sed is not able to correctly parse the script
>>> '/[0-9]/{s,00*,0,g;p}', so break it into two smaller scripts which
>>> FreeBSD (and Linux) sed is able to parse.
>> 
>> First of all - is this again a standard conformance issue? I can't see
>> what's non-conformant with the old approach (which btw had been
>> in place for quite long a time, so I'm puzzled by this being an issue
>> only now), and for future reference purposes it would be nice to
>> know what exactly should be avoided (i.e. to prevent a similar issue
>> from getting introduced again later).
> 
> Sorry, my regex skills are quite basic. The error is the following:
> 
> sed: 1: "/[0-9]/{s,00*,0,g;p}": extra characters at the end of p command

Perhaps there's just a ; missing after the p?

>> And I take it that you adding -r and the use of + isn't really necessary
>> either - the 00* approach should work equally well without using the
>> non-standard (but apparently more wide spread) -r? (Yes, we're just
>> about to commit other uses of -r, but there the alternative would be
>> more difficult to grok, so has better justification.)
> 
> I just thought the use of 0+ was clearer than 00*, but if you prefer to
> avoid using -r in more places the following expression also works on
> FreeBSD and is more similar to the original one:
> 
> sed -ne 's/00*/0/g' -e '/[0-9]/p'
> 
> If you think this is a suitable solution I can resend the patch.

Yeah, I think we should try to avoid using extensions unless this
provides a real benefit.

Jan

Re: [PATCH] build: fix sed usage in build process

[Roger Pau Monné]

On 25/03/14 17:06, Jan Beulich wrote:
>>>> On 25.03.14 at 16:58, <roger.pau@citrix.com> wrote:
>> On 25/03/14 15:06, Jan Beulich wrote:
>>>>>> On 25.03.14 at 12:20, <roger.pau@citrix.com> wrote:
>>>> FreeBSD sed is not able to correctly parse the script
>>>> '/[0-9]/{s,00*,0,g;p}', so break it into two smaller scripts which
>>>> FreeBSD (and Linux) sed is able to parse.
>>>
>>> First of all - is this again a standard conformance issue? I can't see
>>> what's non-conformant with the old approach (which btw had been
>>> in place for quite long a time, so I'm puzzled by this being an issue
>>> only now), and for future reference purposes it would be nice to
>>> know what exactly should be avoided (i.e. to prevent a similar issue
>>> from getting introduced again later).
>>
>> Sorry, my regex skills are quite basic. The error is the following:
>>
>> sed: 1: "/[0-9]/{s,00*,0,g;p}": extra characters at the end of p command
> 
> Perhaps there's just a ; missing after the p?

Certainly, the following works fine:

sed -n '/[0-9]/{s,00*,0,g;p;}'

Since you are the one that found/fixed it, could you please submit the
patch?

Roger.