[Qemu-devel] [PATCH] configure: fix ALSA configure test

[Qemu-devel] [PATCH] configure: fix ALSA configure test

[Igor Mitsyanko]

After commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure tests are
executed with -Werror flag. Current ALSA configure test program invokes a warning:

warning: ‘handle’ is used uninitialized in this function [-Wuninitialized]

which results in error with -Werror flag and, consequently, in alsa test failing.
This means that QEMU won't configure with "--audio-drv-list=alsa".

Initialize "handle" variable to fix compilation warning.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 0a3896e..cf4669f 100755
--- a/configure
+++ b/configure
@@ -1888,7 +1888,7 @@ for drv in $audio_drv_list; do
     case $drv in
     alsa)
     audio_drv_probe $drv alsa/asoundlib.h -lasound \
-        "snd_pcm_t **handle; return snd_pcm_close(*handle);"
+        "snd_pcm_t *handle = 0; return snd_pcm_close(handle);"
     libs_softmmu="-lasound $libs_softmmu"
     ;;
 
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Igor Mitsyanko]

I see now that this bug was already noticed, please ignore this mail

On 07/17/2012 09:34 PM, Igor Mitsyanko wrote:
> After commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure tests are
> executed with -Werror flag. Current ALSA configure test program invokes a warning:
>
> warning: ‘handle’ is used uninitialized in this function [-Wuninitialized]
>
> which results in error with -Werror flag and, consequently, in alsa test failing.
> This means that QEMU won't configure with "--audio-drv-list=alsa".
>
> Initialize "handle" variable to fix compilation warning.
>
> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
> ---
>   configure |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 0a3896e..cf4669f 100755
> --- a/configure
> +++ b/configure
> @@ -1888,7 +1888,7 @@ for drv in $audio_drv_list; do
>       case $drv in
>       alsa)
>       audio_drv_probe $drv alsa/asoundlib.h -lasound \
> -        "snd_pcm_t **handle; return snd_pcm_close(*handle);"
> +        "snd_pcm_t *handle = 0; return snd_pcm_close(handle);"
>       libs_softmmu="-lasound $libs_softmmu"
>       ;;
>
>

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Stefan Weil]

Am 17.07.2012 20:03, schrieb Igor Mitsyanko:
> I see now that this bug was already noticed, please ignore this mail
>
> On 07/17/2012 09:34 PM, Igor Mitsyanko wrote:
>> After commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure 
>> tests are
>> executed with -Werror flag. Current ALSA configure test program 
>> invokes a warning:
>>
>> warning: ‘handle’ is used uninitialized in this function 
>> [-Wuninitialized]
>>
>> which results in error with -Werror flag and, consequently, in alsa 
>> test failing.
>> This means that QEMU won't configure with "--audio-drv-list=alsa".
>>
>> Initialize "handle" variable to fix compilation warning.
>>
>> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
>> ---
>>   configure |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>


Hello Anthony, hello Blue,

these patches fix broken builds, therefore I expected that they
would be applied fast:

http://patchwork.ozlabs.org/patch/171066/
http://patchwork.ozlabs.org/patch/171067/
http://patchwork.ozlabs.org/patch/171081/
http://patchwork.ozlabs.org/patch/171082/

The first of them fixes the ALSA problem.

I think that all 4 patches are trivial, so maybe they could
also be committed via qemu-trivial if the next pull request
comes before they were applied to git master.

Regards,

Stefan W.

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Peter Maydell]

On 17 July 2012 19:32, Stefan Weil <sw@weilnetz.de> wrote:
> Hello Anthony, hello Blue,
>
> these patches fix broken builds, therefore I expected that they
> would be applied fast:
>
> http://patchwork.ozlabs.org/patch/171066/
> http://patchwork.ozlabs.org/patch/171067/
> http://patchwork.ozlabs.org/patch/171081/
> http://patchwork.ozlabs.org/patch/171082/
>
> The first of them fixes the ALSA problem.

I think this is making it clearer that we should back out
the Werror change in favour of something more narrowly
targeted (although we may as well fix these warnings in
test code anyway). Patch to follow...

-- PMM

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Stefan Weil]

Am 17.07.2012 20:46, schrieb Peter Maydell:
> On 17 July 2012 19:32, Stefan Weil <sw@weilnetz.de> wrote:
>> Hello Anthony, hello Blue,
>>
>> these patches fix broken builds, therefore I expected that they
>> would be applied fast:
>>
>> http://patchwork.ozlabs.org/patch/171066/
>> http://patchwork.ozlabs.org/patch/171067/
>> http://patchwork.ozlabs.org/patch/171081/
>> http://patchwork.ozlabs.org/patch/171082/
>>
>> The first of them fixes the ALSA problem.
>
> I think this is making it clearer that we should back out
> the Werror change in favour of something more narrowly
> targeted (although we may as well fix these warnings in
> test code anyway). Patch to follow...
>
> -- PMM

The arguments why -Werror is a bad idea for some configure tests
are reasonable.

Nevertheless the QEMU community was able to produce thousands of
lines of code which compile without a warning, so we should be able
to create warning and error free code for a handful of configure
tests.

The 4 patches above are valid and can be applied with or without
-Werror, therefore qemu-trivial or whoever does not have to wait for
Peter's patch. There is an ongoing discussion for the 3rd patch,
so maybe patches 3 and 4 need to be delayed (they are less important
because they are only needed for some older hosts which
don't use -march=i868 by default).

Regards,
Stefan W.

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Peter Maydell]

On 17 July 2012 20:24, Stefan Weil <sw@weilnetz.de> wrote:
> The arguments why -Werror is a bad idea for some configure tests
> are reasonable.
>
> Nevertheless the QEMU community was able to produce thousands of
> lines of code which compile without a warning, so we should be able
> to create warning and error free code for a handful of configure
> tests.

The trouble is that the warnings and errors here don't cause the
build to fail noisily; that's a big distinction IMHO.
I suppose we could make compile_prog do something like:
 * run the compile test
 * if it fails => test failure as now
 * if it succeeds (and we're doing a Werror build at all),
   rerun the same test with -Werror
 * if that fails, abort configure with an error message
Then we would have the same "make the problem obvious" effect
that plain -Werror provides for our main compilation.

> The 4 patches above are valid and can be applied with or without
> -Werror, therefore qemu-trivial or whoever does not have to wait for
> Peter's patch.

Yes, I agree we might as well fix these errors since we've now
noticed them, regardless of whether or not we apply my patch
(which I've just sent).

-- PMM

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Stefan Weil]

Am 17.07.2012 21:28, schrieb Peter Maydell:
> On 17 July 2012 20:24, Stefan Weil <sw@weilnetz.de> wrote:
>> The arguments why -Werror is a bad idea for some configure tests
>> are reasonable.
>>
>> Nevertheless the QEMU community was able to produce thousands of
>> lines of code which compile without a warning, so we should be able
>> to create warning and error free code for a handful of configure
>> tests.
> The trouble is that the warnings and errors here don't cause the
> build to fail noisily; that's a big distinction IMHO.
> I suppose we could make compile_prog do something like:
>   * run the compile test
>   * if it fails => test failure as now
>   * if it succeeds (and we're doing a Werror build at all),
>     rerun the same test with -Werror
>   * if that fails, abort configure with an error message
> Then we would have the same "make the problem obvious" effect
> that plain -Werror provides for our main compilation.

Good idea. Of course it will increase the time needed for
running the default configure, but I think that's acceptable
if we don't use it for the tests of the compiler warning options.

Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: fix ALSA configure test

[Stefan Hajnoczi]

On Tue, Jul 17, 2012 at 08:28:40PM +0100, Peter Maydell wrote:
> On 17 July 2012 20:24, Stefan Weil <sw@weilnetz.de> wrote:
> > The arguments why -Werror is a bad idea for some configure tests
> > are reasonable.
> >
> > Nevertheless the QEMU community was able to produce thousands of
> > lines of code which compile without a warning, so we should be able
> > to create warning and error free code for a handful of configure
> > tests.
> 
> The trouble is that the warnings and errors here don't cause the
> build to fail noisily; that's a big distinction IMHO.
> I suppose we could make compile_prog do something like:
>  * run the compile test
>  * if it fails => test failure as now
>  * if it succeeds (and we're doing a Werror build at all),
>    rerun the same test with -Werror
>  * if that fails, abort configure with an error message
> Then we would have the same "make the problem obvious" effect
> that plain -Werror provides for our main compilation.
> 
> > The 4 patches above are valid and can be applied with or without
> > -Werror, therefore qemu-trivial or whoever does not have to wait for
> > Peter's patch.
> 
> Yes, I agree we might as well fix these errors since we've now
> noticed them, regardless of whether or not we apply my patch
> (which I've just sent).

Build/configure fixes should go straight into qemu.git.

Stefan

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Blue Swirl]

On Tue, Jul 17, 2012 at 7:28 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 17 July 2012 20:24, Stefan Weil <sw@weilnetz.de> wrote:
>> The arguments why -Werror is a bad idea for some configure tests
>> are reasonable.
>>
>> Nevertheless the QEMU community was able to produce thousands of
>> lines of code which compile without a warning, so we should be able
>> to create warning and error free code for a handful of configure
>> tests.
>
> The trouble is that the warnings and errors here don't cause the
> build to fail noisily; that's a big distinction IMHO.
> I suppose we could make compile_prog do something like:
>  * run the compile test

Unfortunately that would break cross compiling.

>  * if it fails => test failure as now
>  * if it succeeds (and we're doing a Werror build at all),
>    rerun the same test with -Werror
>  * if that fails, abort configure with an error message
> Then we would have the same "make the problem obvious" effect
> that plain -Werror provides for our main compilation.
>
>> The 4 patches above are valid and can be applied with or without
>> -Werror, therefore qemu-trivial or whoever does not have to wait for
>> Peter's patch.
>
> Yes, I agree we might as well fix these errors since we've now
> noticed them, regardless of whether or not we apply my patch
> (which I've just sent).
>
> -- PMM

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Peter Maydell]

On 23 July 2012 18:40, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Jul 17, 2012 at 7:28 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The trouble is that the warnings and errors here don't cause the
>> build to fail noisily; that's a big distinction IMHO.
>> I suppose we could make compile_prog do something like:
>>  * run the compile test
>
> Unfortunately that would break cross compiling.

Sorry, that was slightly unclear phrasing. By "run the test"
I meant "do the check that we can compile the test code"
"run the binary produced". This series won't break cross
compiling.

-- PMM

Re: [Qemu-devel] [PATCH] configure: fix ALSA configure test

[Peter Maydell]

On 23 July 2012 18:45, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 23 July 2012 18:40, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, Jul 17, 2012 at 7:28 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> The trouble is that the warnings and errors here don't cause the
>>> build to fail noisily; that's a big distinction IMHO.
>>> I suppose we could make compile_prog do something like:
>>>  * run the compile test
>>
>> Unfortunately that would break cross compiling.
>
> Sorry, that was slightly unclear phrasing. By "run the test"
> I meant "do the check that we can compile the test code"
> "run the binary produced".

Gah. Missing 'not', should read:
# By "run the test" I meant
# "do the check that we can compile the test code", not
# "run the binary produced".

-- PMM