| Message ID | 1342546459-21993-1-git-send-email-i.mitsyanko@samsung.com |
|---|---|
| State | New |
| Headers | show |
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" > ;; > >
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.
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
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.
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
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.
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
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
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
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
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" ;;
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(-)