[Qemu-devel] [PATCH] Fix OS X SDL breakage

[Qemu-devel] [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1.1: Type: text/plain, Size: 553 bytes --]

Hija,

On OS X vl.c explicitly includes <SDL/SDL.h> which is wrong for two
reasons:
a) the file does compile without the include
b) the path is supposedly incorrect since the include path
    is (should) be set by `sdl-config --cflags` which already
    includes the path "SDL". So this will only work in case
    someone tries to be clever by copying the SDL headers into
    a directory SDL in a standard include directory and breaks
    e.g. on fink where this is not the case.

Attached patch fixes this by removing the seemingly unneeded
include.


[-- Attachment #1.2: diff --]
[-- Type: application/octet-stream, Size: 453 bytes --]

Index: vl.c
===================================================================
RCS file: /cvsroot/qemu/qemu/vl.c,v
retrieving revision 1.117
diff -u -r1.117 vl.c
--- vl.c	15 Jan 2005 21:50:11 -0000	1.117
+++ vl.c	23 Jan 2005 22:26:05 -0000
@@ -66,12 +66,6 @@
 #define memalign(align, size) malloc(size)
 #endif
 
-#ifdef CONFIG_SDL
-#ifdef __APPLE__
-#include <SDL/SDL.h>
-#endif
-#endif /* CONFIG_SDL */
-
 #include "disas.h"
 
 #include "exec-all.h"

[-- Attachment #1.3: Type: text/plain, Size: 23 bytes --]



Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Fabrice Bellard]

Can you confirm that QEMU for OS X works with this include ? IMHO it was 
added to add the '#define main SDL_main' define.

Fabrice.

Daniel Egger wrote:
> Hija,
> 
> On OS X vl.c explicitly includes <SDL/SDL.h> which is wrong for two
> reasons:
> a) the file does compile without the include
> b) the path is supposedly incorrect since the include path
>    is (should) be set by `sdl-config --cflags` which already
>    includes the path "SDL". So this will only work in case
>    someone tries to be clever by copying the SDL headers into
>    a directory SDL in a standard include directory and breaks
>    e.g. on fink where this is not the case.
> 
> Attached patch fixes this by removing the seemingly unneeded
> include.
> 
> 
> 
> Servus,
>       Daniel
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

On 26.01.2005, at 20:47, Fabrice Bellard wrote:

> Can you confirm that QEMU for OS X works with this include ?

I can confirm that the relevant part *compiles* *without* it
i.e. after the applying the patch. Including the include it'll
fail the compilation of vl.o failes because the included file
is not in search path (and cannot be on a sane configuration
I might add).

> IMHO it was added to add the '#define main SDL_main' define.

Why would OS X be any different than say Linux in this regard?

Iff the inclusion of SDL.h was needed, it would be better to
unconditionally #include <SDL.h> on all architectures and fix
the makefile to use `sdl-config --cflags` in addition to the
regular switches for vl.o.

But since OS X still miscompiles the uops the proof is in the
pudding...

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Laurent Amon]

I did not use fink for SDL but did a default install from sources. My  
SDL includes are in /usr/local/include/SDL, so I need the SDL/SDL.h  
lines. Removing them will break the default source install.

Lga.

On 26 janv. 05, at 20:47, Fabrice Bellard wrote:

> Can you confirm that QEMU for OS X works with this include ? IMHO it  
> was added to add the '#define main SDL_main' define.
>
> Fabrice.
>
> Daniel Egger wrote:
>> Hija,
>> On OS X vl.c explicitly includes <SDL/SDL.h> which is wrong for two
>> reasons:
>> a) the file does compile without the include
>> b) the path is supposedly incorrect since the include path
>>    is (should) be set by `sdl-config --cflags` which already
>>    includes the path "SDL". So this will only work in case
>>    someone tries to be clever by copying the SDL headers into
>>    a directory SDL in a standard include directory and breaks
>>    e.g. on fink where this is not the case.
>> Attached patch fixes this by removing the seemingly unneeded
>> include.
>> Servus,
>>       Daniel
>> ---------------------------------------------------------------------- 
>> --
>> _______________________________________________
>> Qemu-devel mailing list
>> Qemu-devel@nongnu.org
>> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>
>

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On 26.01.2005, at 21:58, Laurent Amon wrote:

> I did not use fink for SDL but did a default install from sources. My 
> SDL includes are in /usr/local/include/SDL, so I need the SDL/SDL.h 
> lines. Removing them will break the default source install.

This is wrong. You should use `sdl-config --cflags` instead which
will look return something like -I/usr/local/include/SDL -D_THREAD_SAFE
for your system and thus work in analog to every one elses system.

The current hardcoded include will break for fink users at least
so it is proven that __APPLE__ is not sufficient to characterize
the software installation on a Mac. That's exactly what foo-config
is for.

Anyway; is this include needed at all? Unfortunately it still
doesn't compile through and thus I cannot verify that it links
correctly; but I also do not see a reason why this is needed
at all.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Laurent Amon]

Selon Daniel Egger <de@axiros.com>:

> On 26.01.2005, at 21:58, Laurent Amon wrote:
> This is wrong. You should use `sdl-config --cflags` instead which
> will look return something like -I/usr/local/include/SDL -D_THREAD_SAFE
> for your system and thus work in analog to every one elses system.

Well, whether you install the binary on www.libsdl.org or compile the source,
default install is to put it in a SDL subdirectory. This is a case of fink not
doing the default install. I must say that this is still another of the
grievances I have with fink. It may be useful in some ways, but it breaks a lot
of things (due to the interaction between fink and other packages, I am totally
unable to compile kde, for instance).

Regards,

Lga.

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Johannes Schindelin]

Hi,

On Thu, 27 Jan 2005, Laurent Amon wrote:

> Well, whether you install the binary on www.libsdl.org or compile the
> source, default install is to put it in a SDL subdirectory.

Where do they place sdl-config?

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[René Korthaus]

Am 27.01.2005 um 12:40 schrieb Johannes Schindelin:

> Hi,
>
> On Thu, 27 Jan 2005, Laurent Amon wrote:
>
>> Well, whether you install the binary on www.libsdl.org or compile the
>> source, default install is to put it in a SDL subdirectory.
>
> Where do they place sdl-config?

For Mac OS X, they don't ship the sdl-config anymore. That's at least 
what I read.

>
> Ciao,
> Dscho
>
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>

[Qemu-devel] Re: [PATCH] Fix OS X SDL breakage

[Anand Kumria]

On Thu, 27 Jan 2005 11:51:42 +0100, Laurent Amon wrote:

> Selon Daniel Egger <de@axiros.com>:
> 
>> On 26.01.2005, at 21:58, Laurent Amon wrote: This is wrong. You should
>> use `sdl-config --cflags` instead which will look return something like
>> -I/usr/local/include/SDL -D_THREAD_SAFE for your system and thus work in
>> analog to every one elses system.
> 
> Well, whether you install the binary on www.libsdl.org or compile the
> source, default install is to put it in a SDL subdirectory.

Okay, what is the output of `sdl-config --cflags` on your system?

That would give us some hints as to how to fix things.

Anand

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

On 27.01.2005, at 13:08, René Korthaus wrote:

> For Mac OS X, they don't ship the sdl-config anymore. That's at least 
> what I read.

Who, fink?

lucy:~ egger$ uname -a
Darwin lucy.local 7.7.0 Darwin Kernel Version 7.7.0: Sun Nov  7 
16:06:51 PST 2004; root:xnu/xnu-517.9.5.obj~1/RELEASE_PPC  Power 
Macintosh powerpc

lucy:~ egger$ dpkg -S sdl-config
sdl: /sw/bin/sdl-config

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] Re: [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

On 27.01.2005, at 14:27, Anand Kumria wrote:

> Okay, what is the output of `sdl-config --cflags` on your system?
> That would give us some hints as to how to fix things.

I mentioned this at least two times already, but ok:

lucy:~ egger$ sdl-config --cflags
-I/sw/include/SDL -D_THREAD_SAFE

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] Re: [PATCH] Fix OS X SDL breakage

[Laurent Amon]

On 27 janv. 05, at 18:10, Daniel Egger wrote:

> On 27.01.2005, at 14:27, Anand Kumria wrote:
>
>> Okay, what is the output of `sdl-config --cflags` on your system?
>> That would give us some hints as to how to fix things.
>
> I mentioned this at least two times already, but ok:
>
> lucy:~ egger$ sdl-config --cflags
> -I/sw/include/SDL -D_THREAD_SAFE
>

I have the following (SDL compiled with default options).

lamon@taltos.[~].14%  which sdl-config
/usr/local/bin/sdl-config
lamon@taltos.[~].15% sdl-config --cflags
-I/usr/local/include/SDL -D_THREAD_SAFE

Lga.

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On 27.01.2005, at 11:51, Laurent Amon wrote:

> Well, whether you install the binary on www.libsdl.org or compile the 
> source,
> default install is to put it in a SDL subdirectory. This is a case of 
> fink not
> doing the default install.

This argument is clearly wrong. On Debian the SDL includes will
be installed into /usr/include/SDL, on fink it is /sw/include/SDL.
So the prefix is different, the rest of the path stays the
same.

Anyway, your point is moot, since sdl.c is already compiled with
`sdl-config --cflags` and includes <SDL.h> while vl.c is not
compiled with the correct additional switches as supplied by
sdl-config and includes <SDL/SDL.h> instead. Regardless whether
the include in vl.c is needed at all, the path is clearly wrong.

> I must say that this is still another of the
> grievances I have with fink.

This is *not* the right place to discuss your personal problems
with fink and in this special case that even seemed to blur your
vision on qemu problems...

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] Re: [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

On 27.01.2005, at 20:34, Laurent Amon wrote:

> I have the following (SDL compiled with default options).
>
> lamon@taltos.[~].14%  which sdl-config
> /usr/local/bin/sdl-config
> lamon@taltos.[~].15% sdl-config --cflags
> -I/usr/local/include/SDL -D_THREAD_SAFE

So #include <SDL/SDL.h> would work per pure luck for you because
/usr/local/include is in the standard include search path on OS X.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

On 27.01.2005, at 13:08, René Korthaus wrote:

> For Mac OS X, they don't ship the sdl-config anymore. That's at least 
> what I read.

Who is "they"?
What is "ship" supposed to mean in this context?
How is this related to one *obviously* wrong or 5 obsolete
lines of code?
Where the source of such FUD?

People, this is getting really ridiculous; all the arguing about
something this trivial and clearly wrong. Who's the author of the
lines? Maybe this person can shed a bit light on it.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Laurent Amon]

On 27 janv. 05, at 13:08, René Korthaus wrote:
> For Mac OS X, they don't ship the sdl-config anymore. That's at least 
> what I read.
I just recompiled 1.2.8 and sdl-config gets installed if you compile it 
the unix way (which is the way qemu uses it).
I believe that if you use Xcode to compile a SDL framework, you won't 
get it --- and i is not needed. But Qemu won't be able to use it.

Lga.

Re: [Qemu-devel] [PATCH] Fix OS X SDL breakage

[Bernhard Ehlers]

Fabrice Bellard wrote:

> Can you confirm that QEMU for OS X works with this include ? IMHO it was
> added to add the '#define main SDL_main' define.
> 
> Fabrice.

Hi,

Here my test results on my Mac OS X 10.3.7.

I use the qemu-snapshot-2004-12-28_23.tar.bz2 from http://people.fruitsalad.org/nox/ , but the results should be the same on CVS.

1)  self compiled SDL V1.2.8

# sdl-config --version --cflags --libs
1.2.8
-I/usr/local/include/SDL -D_THREAD_SAFE
-L/usr/local/lib -lSDLmain -lSDL -framework Cocoa -framework OpenGL

a) no change to vl.c
compiles OK, links OK, runs OK

b) remove the #include SDL/SDL.h in vl.c
compiles OK, links OK, crashes during execution

2005-01-29 11:31:55.502 qemu[1222] *** _NSAutoreleaseNoPool(): Object 0x1b01550 of class NSCFArray autoreleased with no pool in place - just leaking
.
.
.
2005-01-29 11:31:55.533 qemu[1222] *** _NSAutoreleaseNoPool(): Object 0x1b39850 of class NSException autoreleased with no pool in place - just leaking
2005-01-29 11:31:55.533 qemu[1222] *** Uncaught exception: <NSInternalInconsistencyException> Error (1002) creating CGSWindow

2)  Fink sdl v1.2.7-1

# sdl-config --version --cflags --libs
1.2.7
-I/sw/include/SDL -D_THREAD_SAFE
-L/sw/lib -lSDLmain -lSDL -framework Cocoa -framework OpenGL

a) no change to vl.c
compile error in vl.c, include file SDL/SDL.h not found

b) remove the #include SDL/SDL.h in vl.c
compiles OK, links OK, crashes during execution (see 1b)


So the removal of the #include <SDL/SDL.h> crashes the executable on MAC OSX. That means it has to stay.

On the other side it compiles only for the default SDL installation because /usr/local/include is in the default include search path. The problem is, that for the compilation of vl.c the SDL include path (sdl-config --cflags) is not used.

My proposal is to modify the Makefiles, so that for the compilation of vl.c the SDL_CFLAGS are used.

Here my patches. Now it compiles, links and runs with both SDL libraries.

Best regards

Bernhard Ehlers

diff -ru qemu.orig/Makefile.target qemu/Makefile.target
--- qemu.orig/Makefile.target	Mon Dec 20 00:33:47 2004
+++ qemu/Makefile.target	Sat Jan 29 12:16:04 2005
@@ -348,6 +348,9 @@
 sdlaudio.o: sdlaudio.c
 	$(CC) $(CFLAGS) $(DEFINES) $(SDL_CFLAGS) -c -o $@ $<
 
+vl.o: vl.c vl.h
+	$(CC) $(CFLAGS) $(DEFINES) $(SDL_CFLAGS) -c -o $@ $<
+
 depend: $(SRCS)
 	$(CC) -MM $(CFLAGS) $(DEFINES) $^ 1>.depend
 
diff -ru qemu.orig/vl.c qemu/vl.c
--- qemu.orig/vl.c	Mon Dec 20 00:18:01 2004
+++ qemu/vl.c	Sat Jan 29 12:16:34 2005
@@ -68,7 +68,7 @@
 
 #ifdef CONFIG_SDL
 #ifdef __APPLE__
-#include <SDL/SDL.h>
+#include <SDL.h>
 #endif
 #endif /* CONFIG_SDL */