[Qemu-devel] [PATCH] configure: don't try to compile against known broken curses.

[Qemu-devel] [PATCH] configure: don't try to compile against known broken curses.

[Andrzej Zaborowski]

This should resolve a problem noted by Caraman Mihai Claudiu.

Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>
---
 configure |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 61c43b9..678b982 100755
--- a/configure
+++ b/configure
@@ -1846,6 +1846,11 @@ if test "$curses" != "no" ; then
 #ifdef __OpenBSD__
 #define resize_term resizeterm
 #endif
+#ifdef NCURSES_VERSION
+# if NCURSES_VERSION_PATCH < 20040117
+#  error Old ncurses contain dangerous typedefs, break qemu build (and are old)
+# endif
+#endif
 int main(void) { resize_term(0, 0); return curses_version(); }
 EOF
   for curses_lib in $curses_list; do
-- 
1.7.4.4

[Qemu-devel] [PATCH 1/2] configure: don't check for Cocoa when detecting SDL.

[Andrzej Zaborowski]

The SDL check is supposed to set $sdl to "yes" or "no", but with that
check it leaves $sdl unset on darwin, unless --enable-cocoa was
specified (which is not needed to enable cocoa anyway).

Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>
---
 configure |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 678b982..fb15bc6 100755
--- a/configure
+++ b/configure
@@ -1492,9 +1492,7 @@ EOF
     if test "$_sdlversion" -lt 121 ; then
       sdl_too_old=yes
     else
-      if test "$cocoa" = "no" ; then
-        sdl=yes
-      fi
+      sdl=yes
     fi
 
     # static link with sdl ? (note: sdl.pc's --static --libs is broken)
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 1/2] configure: don't check for Cocoa when detecting SDL.

[Andreas Färber]

Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
> The SDL check is supposed to set $sdl to "yes" or "no", but with that
> check it leaves $sdl unset on darwin, unless --enable-cocoa was
> specified (which is not needed to enable cocoa anyway).
> 
> Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>
> ---
>  configure |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 678b982..fb15bc6 100755
> --- a/configure
> +++ b/configure
> @@ -1492,9 +1492,7 @@ EOF
>      if test "$_sdlversion" -lt 121 ; then
>        sdl_too_old=yes
>      else
> -      if test "$cocoa" = "no" ; then
> -        sdl=yes
> -      fi
> +      sdl=yes

With no further explanation in the commit description I have doubts if
this is really "safe". Cocoa actually masquerades as SDL in vl.c and has
no display type of its own, so there may well be a real need to suppress
SDL in the --enable-cocoa case. The #ifdef'fery may need to be sanitized
first, at least fully reviewed.

Also please cc me on Cocoa patches.

Thanks,
Andreas

>      fi
>  
>      # static link with sdl ? (note: sdl.pc's --static --libs is broken)

[Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[Andrzej Zaborowski]

Cocoa can only be enabled on Darwin, and is enabled by default too,
making --enable-cocoa redundant, with no way to disable Cocoa.  It
also interfered with SDL support in a way that was dependent on
the order of commandline switches.

Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>
---
Cocoa support seems to be broken at the moment, at least on some
MacOS X versions.  But qemu builds and runs with SDL.

 configure |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index fb15bc6..c5d07af 100755
--- a/configure
+++ b/configure
@@ -674,10 +674,7 @@ for opt do
   ;;
   --enable-profiler) profiler="yes"
   ;;
-  --enable-cocoa)
-      cocoa="yes" ;
-      sdl="no" ;
-      audio_drv_list="coreaudio `echo $audio_drv_list | sed s,coreaudio,,g`"
+  --disable-cocoa) cocoa="no"
   ;;
   --disable-system) softmmu="no"
   ;;
@@ -986,7 +983,7 @@ echo "  --disable-sdl            disable SDL"
 echo "  --enable-sdl             enable SDL"
 echo "  --disable-vnc            disable VNC"
 echo "  --enable-vnc             enable VNC"
-echo "  --enable-cocoa           enable COCOA (Mac OS X only)"
+echo "  --disable-cocoa          disable COCOA (Mac OS X only)"
 echo "  --audio-drv-list=LIST    set audio drivers list:"
 echo "                           Available drivers: $audio_possible_drivers"
 echo "  --audio-card-list=LIST   set list of emulated audio cards [$audio_card_list]"
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[Peter Maydell]

On 7 December 2011 07:47, Andrzej Zaborowski <balrogg@gmail.com> wrote:
> Cocoa can only be enabled on Darwin, and is enabled by default too,
> making --enable-cocoa redundant, with no way to disable Cocoa.  It
> also interfered with SDL support in a way that was dependent on
> the order of commandline switches.

For these --enable/disable pairs I quite like the pattern where
 * default is "probe and use if available"
 * --enable-foo is "use foo, fail configure if not available"
 * --disable-foo is "don't use foo"

(--enable-sdl/--disable-sdl work like this, for instance).

[cf the comment in configure at line 100.]

Incidentally, is it "Cocoa" or "COCOA" ?

-- PMM

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[andrzej zaborowski]

On 7 December 2011 19:56, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 December 2011 07:47, Andrzej Zaborowski <balrogg@gmail.com> wrote:
>> Cocoa can only be enabled on Darwin, and is enabled by default too,
>> making --enable-cocoa redundant, with no way to disable Cocoa.  It
>> also interfered with SDL support in a way that was dependent on
>> the order of commandline switches.
>
> For these --enable/disable pairs I quite like the pattern where
>  * default is "probe and use if available"
>  * --enable-foo is "use foo, fail configure if not available"
>  * --disable-foo is "don't use foo"
>
> (--enable-sdl/--disable-sdl work like this, for instance).

Yep, the difference here is that there's no probing, so --enable is a
little redundant, but maybe for consistency it's better to have
anyway.  One of the issues with current --enable-cocoa though is that
it has a different effect than if Cocoa is enabled by default.

>
> [cf the comment in configure at line 100.]
>
> Incidentally, is it "Cocoa" or "COCOA" ?

It appears as Cocoa in the system libraries, but don't take my word.

Cheers

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[Andreas Färber]

Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
> Cocoa can only be enabled on Darwin, and is enabled by default too,
> making --enable-cocoa redundant, with no way to disable Cocoa.  It
> also interfered with SDL support in a way that was dependent on
> the order of commandline switches.
> 
> Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>

Nack. This not only conflicts with Pavel's patch series but like many
previous patches only does half the job (misses the block layer).
Could you please review his last series instead and rebase onto that if
necessary?

http://patchwork.ozlabs.org/patch/124980/
http://patchwork.ozlabs.org/patch/124979/
http://patchwork.ozlabs.org/patch/124981/

> ---
> Cocoa support seems to be broken at the moment, at least on some
> MacOS X versions.  But qemu builds and runs with SDL.

Many times have I asked how to actually use SDL with QEMU on Mac OS X.
If you've figured it out, please share that knowledge! What SDL download
do you use, what parameters do you pass to configure, etc.?

Note that I have a patch that replaces uint16 with uint_fast16_t,
properly fixing the Cocoa build. What I don't have yet is all the other
conversions (Coccinelle doesn't fully do int16 conversion, for example)
to run the benchmarks Peter asked for.

Andreas

>  configure |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/configure b/configure
> index fb15bc6..c5d07af 100755
> --- a/configure
> +++ b/configure
> @@ -674,10 +674,7 @@ for opt do
>    ;;
>    --enable-profiler) profiler="yes"
>    ;;
> -  --enable-cocoa)
> -      cocoa="yes" ;
> -      sdl="no" ;
> -      audio_drv_list="coreaudio `echo $audio_drv_list | sed s,coreaudio,,g`"
> +  --disable-cocoa) cocoa="no"
>    ;;
>    --disable-system) softmmu="no"
>    ;;
> @@ -986,7 +983,7 @@ echo "  --disable-sdl            disable SDL"
>  echo "  --enable-sdl             enable SDL"
>  echo "  --disable-vnc            disable VNC"
>  echo "  --enable-vnc             enable VNC"
> -echo "  --enable-cocoa           enable COCOA (Mac OS X only)"
> +echo "  --disable-cocoa          disable COCOA (Mac OS X only)"
>  echo "  --audio-drv-list=LIST    set audio drivers list:"
>  echo "                           Available drivers: $audio_possible_drivers"
>  echo "  --audio-card-list=LIST   set list of emulated audio cards [$audio_card_list]"

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[Peter Maydell]

On 7 December 2011 21:12, Andreas Färber <andreas.faerber@web.de> wrote:
> Note that I have a patch that replaces uint16 with uint_fast16_t,
> properly fixing the Cocoa build. What I don't have yet is all the other
> conversions (Coccinelle doesn't fully do int16 conversion, for example)
> to run the benchmarks Peter asked for.

For the benchmarks surely it suffices to flip the typedefs, ie compare
typedef uint8_t uint8;
typedef int8_t int8;
typedef uint16_t uint16;
typedef int16_t int16;
typedef uint32_t uint32;
typedef int32_t int32;
typedef uint64_t uint64;
typedef int64_t int64;

with
typedef uint_fast8_t uint8;
typedef int_fast8_t int8;
typedef uint_fast16_t uint16;
typedef int_fast16_t int16;
typedef uint_fast32_t uint32;
typedef int_fast32_t int32;
typedef uint_fast64_t uint64;
typedef int_fast64_t int64;

?

We only need to do the full search-n-replace when we've picked
which one we're going for...

-- PMM

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[Andreas Färber]

Am 07.12.2011 22:21, schrieb Peter Maydell:
> On 7 December 2011 21:12, Andreas Färber <andreas.faerber@web.de> wrote:
>> Note that I have a patch that replaces uint16 with uint_fast16_t,
>> properly fixing the Cocoa build. What I don't have yet is all the other
>> conversions (Coccinelle doesn't fully do int16 conversion, for example)
>> to run the benchmarks Peter asked for.
> 
> For the benchmarks surely it suffices to flip the typedefs, ie compare
> typedef uint8_t uint8;
> typedef int8_t int8;
> typedef uint16_t uint16;
> typedef int16_t int16;
> typedef uint32_t uint32;
> typedef int32_t int32;
> typedef uint64_t uint64;
> typedef int64_t int64;
> 
> with
> typedef uint_fast8_t uint8;
> typedef int_fast8_t int8;
> typedef uint_fast16_t uint16;
> typedef int_fast16_t int16;
> typedef uint_fast32_t uint32;
> typedef int_fast32_t int32;
> typedef uint_fast64_t uint64;
> typedef int_fast64_t int64;
> 
> ?
> 
> We only need to do the full search-n-replace when we've picked
> which one we're going for...

FWIW, target-mips/cpu.h has this:

// uint_fast8_t and uint_fast16_t not in <sys/int_types.h>
// XXX: move that elsewhere
#if defined(CONFIG_SOLARIS) && CONFIG_SOLARIS_VERSION < 10
typedef unsigned char           uint_fast8_t;
typedef unsigned int            uint_fast16_t;
#endif

This shouldn't stop us from using these types, on the contrary, there is
prior art. We'd just have to move these to qemu-common.h or so.

We still don't build on OpenIndiana due to -std=gnu99 vs.
make_floatx80() initialization code BTW. Any ideas there appreciated.

Andreas

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[andrzej zaborowski]

On 7 December 2011 22:12, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
>> Cocoa can only be enabled on Darwin, and is enabled by default too,
>> making --enable-cocoa redundant, with no way to disable Cocoa.  It
>> also interfered with SDL support in a way that was dependent on
>> the order of commandline switches.
>>
>> Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>
>
> Nack. This not only conflicts with Pavel's patch series but like many
> previous patches only does half the job (misses the block layer).

Depends which job you're talking about :)

Sorry, I wasn't aware Cocoa was used by anything other than the UI or
aware of Pavel's patches.

> Could you please review his last series instead and rebase onto that if
> necessary?
>
> http://patchwork.ozlabs.org/patch/124980/
> http://patchwork.ozlabs.org/patch/124979/
> http://patchwork.ozlabs.org/patch/124981/

They look fine to me but I know very little about MacOS X and its libraries.

>
>> ---
>> Cocoa support seems to be broken at the moment, at least on some
>> MacOS X versions.  But qemu builds and runs with SDL.
>
> Many times have I asked how to actually use SDL with QEMU on Mac OS X.
> If you've figured it out, please share that knowledge! What SDL download
> do you use, what parameters do you pass to configure, etc.?

I installed SDL through "fink", the package manager.  It's sdl_1.2.4-8
and the configure line apparently is nothing more than ./configure
--prefix=/sw --mandir=/sw/share/man

Last time I was using it with 0.14 with no issues, now that I rebuilt
HEAD with SDL, I see two issues which may be related to SDL or not:

* blue component is 0, so stuff is yellowish on the screen.
* ctrl-alt-<number> doesn't work, while ctr-alt does.

Cheers

Re: [Qemu-devel] [PATCH 2/2] configure: remove --enable-cocoa (default), add --disable-cocoa.

[andrzej zaborowski]

On 9 December 2011 02:25, andrzej zaborowski <balrogg@gmail.com> wrote:
>>> Cocoa support seems to be broken at the moment, at least on some
>>> MacOS X versions.  But qemu builds and runs with SDL.
>>
>> Many times have I asked how to actually use SDL with QEMU on Mac OS X.
>> If you've figured it out, please share that knowledge! What SDL download
>> do you use, what parameters do you pass to configure, etc.?
>
> I installed SDL through "fink", the package manager.  It's sdl_1.2.4-8
> and the configure line apparently is nothing more than ./configure
> --prefix=/sw --mandir=/sw/share/man

Fink applies the following patch, too:
http://fink.cvs.sourceforge.net/viewvc/fink/dists/10.7/stable/main/finkinfo/games/sdl.patch?revision=1.2&view=markup

Cheers

Re: [Qemu-devel] [PATCH] configure: don't try to compile against known broken curses.

[Stefan Weil]

Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
> This should resolve a problem noted by Caraman Mihai Claudiu.
>
> Signed-off-by: Andrzej Zaborowski <andrew.zaborowski@intel.com>
> ---
> configure | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/configure b/configure
> index 61c43b9..678b982 100755
> --- a/configure
> +++ b/configure
> @@ -1846,6 +1846,11 @@ if test "$curses" != "no" ; then
> #ifdef __OpenBSD__
> #define resize_term resizeterm
> #endif
> +#ifdef NCURSES_VERSION
> +# if NCURSES_VERSION_PATCH < 20040117
> +# error Old ncurses contain dangerous typedefs, break qemu build (and 
> are old)
> +# endif
> +#endif
> int main(void) { resize_term(0, 0); return curses_version(); }
> EOF
> for curses_lib in $curses_list; do

Is NCURSES_VERSION_PATCH always defined when NCURSES_VERSION is?
If not, code like the following is more robust:

#if !defined(NCURSES_VERSION_PATCH) || NCURSES_VERSION_PATCH < 20040117
# error Old ncurses contain dangerous typedefs, break qemu build (and 
are old)
#endif

Otherwise the patch is good.

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [Qemu-devel] [PATCH] configure: don't try to compile against known broken curses.

[andrzej zaborowski]

On 7 December 2011 19:57, Stefan Weil <sw@weilnetz.de> wrote:
> Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
>> +#ifdef NCURSES_VERSION
>> +# if NCURSES_VERSION_PATCH < 20040117
>> +# error Old ncurses contain dangerous typedefs, break qemu build (and are
>> old)
>> +# endif
>> +#endif
>> int main(void) { resize_term(0, 0); return curses_version(); }
>> EOF
>> for curses_lib in $curses_list; do
>
>
> Is NCURSES_VERSION_PATCH always defined when NCURSES_VERSION is?

I'm not sure, will try to find out.  If it isn't then we should check
that NCURSES_VERSION_MINOR < 4 perhaps.

The intent of checking defined(NCURSES_VERSION) is to detect ncurses
because qemu should also build with other implementations of curses
(in theory).

Cheers

Re: [Qemu-devel] [PATCH] configure: don't try to compile against known broken curses.

[Stefan Weil]

Am 07.12.2011 20:06, schrieb andrzej zaborowski:
> On 7 December 2011 19:57, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
>>> +#ifdef NCURSES_VERSION
>>> +# if NCURSES_VERSION_PATCH < 20040117
>>> +# error Old ncurses contain dangerous typedefs, break qemu build 
>>> (and are
>>> old)
>>> +# endif
>>> +#endif
>>> int main(void) { resize_term(0, 0); return curses_version(); }
>>> EOF
>>> for curses_lib in $curses_list; do
>>
>>
>> Is NCURSES_VERSION_PATCH always defined when NCURSES_VERSION is?
>
> I'm not sure, will try to find out. If it isn't then we should check
> that NCURSES_VERSION_MINOR < 4 perhaps.
>
> The intent of checking defined(NCURSES_VERSION) is to detect ncurses
> because qemu should also build with other implementations of curses
> (in theory).
>
> Cheers

Yes, that's right. W32 for example supports pdcurses. So the check
might look like this:

#if defined(NCURSES_VERSION_PATCH) && NCURSES_VERSION_PATCH < 20040117
# error Old ncurses contain dangerous typedefs, break qemu build (and 
are old)
#endif

Cheers,
Stefan

Re: [Qemu-devel] [PATCH] configure: don't try to compile against known broken curses.

[andrzej zaborowski]

On 7 December 2011 20:06, andrzej zaborowski <balrogg@gmail.com> wrote:
> On 7 December 2011 19:57, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 07.12.2011 08:47, schrieb Andrzej Zaborowski:
>>> +#ifdef NCURSES_VERSION
>>> +# if NCURSES_VERSION_PATCH < 20040117
>>> +# error Old ncurses contain dangerous typedefs, break qemu build (and are
>>> old)
>>> +# endif
>>> +#endif
>>> int main(void) { resize_term(0, 0); return curses_version(); }
>>> EOF
>>> for curses_lib in $curses_list; do
>>
>>
>> Is NCURSES_VERSION_PATCH always defined when NCURSES_VERSION is?
>
> I'm not sure, will try to find out.  If it isn't then we should check
> that NCURSES_VERSION_MINOR < 4 perhaps.

At least since 4.2 (19980211) both are defined, I'll apply a version
of the patch similar to what you proposed.

Cheers