Patchwork configure: Fix evaluation of config-host.mak in create_config

login
register
mail settings
Submitter Jan Kiszka
Date June 11, 2010, 8:58 p.m.
Message ID <4C12A375.6050201@web.de>
Download mbox | patch
Permalink /patch/55370/
State New
Headers show

Comments

Jan Kiszka - June 11, 2010, 8:58 p.m.
From: Jan Kiszka <jan.kiszka@siemens.com>

Only match on true dir variable assignments, avoid generating garbage
due to the "# Configured with: ..." line which may contain "*dir=" as
well.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 create_config |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Aurelien Jarno - June 12, 2010, 11:55 a.m.
On Fri, Jun 11, 2010 at 10:58:29PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Only match on true dir variable assignments, avoid generating garbage
> due to the "# Configured with: ..." line which may contain "*dir=" as
> well.

Wouldn't it be better to skip all lines starting with '#', as this
problem might happen again after some more changes?

Or maybe we should do both.

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  create_config |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/create_config b/create_config
> index 23c0cd5..0098e68 100755
> --- a/create_config
> +++ b/create_config
> @@ -13,7 +13,7 @@ case $line in
>      pkgversion=${line#*=}
>      echo "#define QEMU_PKGVERSION \"$pkgversion\""
>      ;;
> - prefix=* | *dir=*) # directory configuration
> + prefix=* | [a-z]*dir=*) # directory configuration
>      name=${line%=*}
>      value=${line#*=}
>      define_name=`echo $name | tr '[:lower:]' '[:upper:]'`
> 
>
Jan Kiszka - June 12, 2010, 12:25 p.m.
Aurelien Jarno wrote:
> On Fri, Jun 11, 2010 at 10:58:29PM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Only match on true dir variable assignments, avoid generating garbage
>> due to the "# Configured with: ..." line which may contain "*dir=" as
>> well.
> 
> Wouldn't it be better to skip all lines starting with '#', as this
> problem might happen again after some more changes?
> 
> Or maybe we should do both.

This is what the patch (implicitly) does.

Jan

> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  create_config |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/create_config b/create_config
>> index 23c0cd5..0098e68 100755
>> --- a/create_config
>> +++ b/create_config
>> @@ -13,7 +13,7 @@ case $line in
>>      pkgversion=${line#*=}
>>      echo "#define QEMU_PKGVERSION \"$pkgversion\""
>>      ;;
>> - prefix=* | *dir=*) # directory configuration
>> + prefix=* | [a-z]*dir=*) # directory configuration
>>      name=${line%=*}
>>      value=${line#*=}
>>      define_name=`echo $name | tr '[:lower:]' '[:upper:]'`
>>
>>
>
Aurelien Jarno - June 12, 2010, 2:37 p.m.
On Sat, Jun 12, 2010 at 02:25:10PM +0200, Jan Kiszka wrote:
> Aurelien Jarno wrote:
> > On Fri, Jun 11, 2010 at 10:58:29PM +0200, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Only match on true dir variable assignments, avoid generating garbage
> >> due to the "# Configured with: ..." line which may contain "*dir=" as
> >> well.
> > 
> > Wouldn't it be better to skip all lines starting with '#', as this
> > problem might happen again after some more changes?
> > 
> > Or maybe we should do both.
> 
> This is what the patch (implicitly) does.

Yes, but only for this line. What I mean is something more generic
like:

"#"*) # comment
    continue
    ;;
Jan Kiszka - June 12, 2010, 4:44 p.m.
Aurelien Jarno wrote:
> On Sat, Jun 12, 2010 at 02:25:10PM +0200, Jan Kiszka wrote:
>> Aurelien Jarno wrote:
>>> On Fri, Jun 11, 2010 at 10:58:29PM +0200, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> Only match on true dir variable assignments, avoid generating garbage
>>>> due to the "# Configured with: ..." line which may contain "*dir=" as
>>>> well.
>>> Wouldn't it be better to skip all lines starting with '#', as this
>>> problem might happen again after some more changes?
>>>
>>> Or maybe we should do both.
>> This is what the patch (implicitly) does.
> 
> Yes, but only for this line. What I mean is something more generic
> like:
> 
> "#"*) # comment
>     continue
>     ;;
> 

IMO, that would only paper over suboptimally expressed patterns for the
other line evaluations.

Jan
Paolo Bonzini - June 12, 2010, 5:57 p.m.
On 06/12/2010 06:44 PM, Jan Kiszka wrote:
>> Yes, but only for this line. What I mean is something more generic
>> like:
>>
>> "#"*) # comment
>>      continue
>>      ;;
>>
>
> IMO, that would only paper over suboptimally expressed patterns for the
> other line evaluations.

Agreed.  I like Jan's patch more.

Paolo
Aurelien Jarno - June 13, 2010, 10:50 a.m.
On Sat, Jun 12, 2010 at 07:57:52PM +0200, Paolo Bonzini wrote:
> On 06/12/2010 06:44 PM, Jan Kiszka wrote:
> >>Yes, but only for this line. What I mean is something more generic
> >>like:
> >>
> >>"#"*) # comment
> >>     continue
> >>     ;;
> >>
> >
> >IMO, that would only paper over suboptimally expressed patterns for the
> >other line evaluations.
> 
> Agreed.  I like Jan's patch more.
> 

I have applied it.

Patch

diff --git a/create_config b/create_config
index 23c0cd5..0098e68 100755
--- a/create_config
+++ b/create_config
@@ -13,7 +13,7 @@  case $line in
     pkgversion=${line#*=}
     echo "#define QEMU_PKGVERSION \"$pkgversion\""
     ;;
- prefix=* | *dir=*) # directory configuration
+ prefix=* | [a-z]*dir=*) # directory configuration
     name=${line%=*}
     value=${line#*=}
     define_name=`echo $name | tr '[:lower:]' '[:upper:]'`