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

["Roger Pau Monné" <roger.pau@citrix.com>]

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.