[Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Stefan Weil]

"+=" does not work with dash and other simple /bin/sh implementations.

The new code prepends the flag while the old code either did not work
(it continued after an error message which typically was not read) or
appended the flag. That difference should not matter here.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 12351f5..0269ba0 100755
--- a/configure
+++ b/configure
@@ -2822,7 +2822,7 @@ int main(int argc, char **argv)
 }
 EOF
   if ! compile_prog "" "" ; then
-    CFLAGS+="-march=i486"
+    CFLAGS="-march=i486 $CFLAGS"
   fi
 fi
 
-- 
1.7.0.4

[Qemu-devel] [PATCH 2/2] configure: Fix errors in test for__sync_fetch_and_and

[Stefan Weil]

The old test code raises two compiler warnings which are errors since
commit 417c9d72d48275d19c60861896efd4962d21aca2.

These errors could result in compilations with compiler flag
-march486 (so all nice features of newer processors got lost).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 0269ba0..df5c99a 100755
--- a/configure
+++ b/configure
@@ -2809,7 +2809,7 @@ fi
 # specification is necessary
 if test "$vhost_net" = "yes" && test "$cpu" = "i386"; then
   cat > $TMPC << EOF
-int sfaa(unsigned *ptr)
+static int sfaa(int *ptr)
 {
   return __sync_fetch_and_and(ptr, 0);
 }
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Peter Maydell]

On 15 July 2012 19:34, Stefan Weil <sw@weilnetz.de> wrote:
> "+=" does not work with dash and other simple /bin/sh implementations.
>
> The new code prepends the flag while the old code either did not work
> (it continued after an error message which typically was not read) or
> appended the flag. That difference should not matter here.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  configure |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 12351f5..0269ba0 100755
> --- a/configure
> +++ b/configure
> @@ -2822,7 +2822,7 @@ int main(int argc, char **argv)
>  }
>  EOF
>    if ! compile_prog "" "" ; then
> -    CFLAGS+="-march=i486"
> +    CFLAGS="-march=i486 $CFLAGS"
>    fi
>  fi

This is not quite the right fix for this. This flag should be in
QEMU_CFLAGS, because it is a flag without which QEMU
would be unable to compile. See previous discussion in this
thread:

http://lists.xen.org/archives/html/xen-devel/2012-04/msg00330.html

(Unfortunately Olaf never submitted an updated patch.)

-- PMM

Re: [Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Stefan Weil]

Am 17.07.2012 20:43, schrieb Peter Maydell:
> On 15 July 2012 19:34, Stefan Weil <sw@weilnetz.de> wrote:
>> "+=" does not work with dash and other simple /bin/sh implementations.
>>
>> The new code prepends the flag while the old code either did not work
>> (it continued after an error message which typically was not read) or
>> appended the flag. That difference should not matter here.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>   configure |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 12351f5..0269ba0 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2822,7 +2822,7 @@ int main(int argc, char **argv)
>>   }
>>   EOF
>>     if ! compile_prog "" "" ; then
>> -    CFLAGS+="-march=i486"
>> +    CFLAGS="-march=i486 $CFLAGS"
>>     fi
>>   fi
>
> This is not quite the right fix for this. This flag should be in
> QEMU_CFLAGS, because it is a flag without which QEMU
> would be unable to compile. See previous discussion in this
> thread:
>
> http://lists.xen.org/archives/html/xen-devel/2012-04/msg00330.html
>
> (Unfortunately Olaf never submitted an updated patch.)
>
> -- PMM
>

If the user overrides CFLAGS, I expect that he/she will notice
that compilation fails and hopefully find the cause (only
expert users should override CFLAGS). Overriding -march=i486
might be useful to set -march=i686.

Therefore it was not so obvious to me whether CFLAGS or
QEMU_CFLAGS was the better choice.

I don't mind if we decide to use QEMU_CFLAGS here,
but suggest putting that change in a separate patch.

This one fixes a shell syntax error without changing the macro name.

Regards,

Stefan W.

Re: [Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Peter Maydell]

On 17 July 2012 20:07, Stefan Weil <sw@weilnetz.de> wrote:
> If the user overrides CFLAGS, I expect that he/she will notice
> that compilation fails and hopefully find the cause (only
> expert users should override CFLAGS). Overriding -march=i486
> might be useful to set -march=i686.
>
> Therefore it was not so obvious to me whether CFLAGS or
> QEMU_CFLAGS was the better choice.

Yes, I had to dig to find out the underlying principle
distinguishing the two. Having found it I would like us to
fix this in a way which respects that principle.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Andreas Färber]

Am 17.07.2012 21:19, schrieb Peter Maydell:
> On 17 July 2012 20:07, Stefan Weil <sw@weilnetz.de> wrote:
>> If the user overrides CFLAGS, I expect that he/she will notice
>> that compilation fails and hopefully find the cause (only
>> expert users should override CFLAGS). Overriding -march=i486
>> might be useful to set -march=i686.
>>
>> Therefore it was not so obvious to me whether CFLAGS or
>> QEMU_CFLAGS was the better choice.
> 
> Yes, I had to dig to find out the underlying principle
> distinguishing the two. Having found it I would like us to
> fix this in a way which respects that principle.

I was planning to pick up Olaf's patch doing just that but haven't got
around to it in time for v1.1. Olaf, do you have time to resubmit a
patch to upstream qemu-devel as requested by Peter and me?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Peter Maydell]

On 20 July 2012 13:28, Andreas Färber <afaerber@suse.de> wrote:
> Am 17.07.2012 21:19, schrieb Peter Maydell:
>> On 17 July 2012 20:07, Stefan Weil <sw@weilnetz.de> wrote:
>>> If the user overrides CFLAGS, I expect that he/she will notice
>>> that compilation fails and hopefully find the cause (only
>>> expert users should override CFLAGS). Overriding -march=i486
>>> might be useful to set -march=i686.
>>>
>>> Therefore it was not so obvious to me whether CFLAGS or
>>> QEMU_CFLAGS was the better choice.
>>
>> Yes, I had to dig to find out the underlying principle
>> distinguishing the two. Having found it I would like us to
>> fix this in a way which respects that principle.
>
> I was planning to pick up Olaf's patch doing just that but haven't got
> around to it in time for v1.1. Olaf, do you have time to resubmit a
> patch to upstream qemu-devel as requested by Peter and me?

I actually wrote a patch sitting on top of this one by Stefan
which moves the flag to QEMU_CFLAGS;
both are in the 11-patch series dealing with various configure
Werror issues:
 http://patchwork.ozlabs.org/patch/171689/
 http://patchwork.ozlabs.org/patch/171706/

-- PMM

Re: [Qemu-devel] [PATCH 1/2] configure: Replace bash code by standard shell code

[Olaf Hering]

On Fri, Jul 20, Peter Maydell wrote:

> I actually wrote a patch sitting on top of this one by Stefan
> which moves the flag to QEMU_CFLAGS;
> both are in the 11-patch series dealing with various configure
> Werror issues:
>  http://patchwork.ozlabs.org/patch/171689/

The actual CFLAGS issue had to be solved differently in the xen sources.
And the patch above solves the bug in configure.

Olaf