[Qemu-devel] [PATCH v2 0/4] Cocoa patches for 1.0

[Qemu-devel] [PATCH v2 0/4] Cocoa patches for 1.0

[Andreas Färber]

Hello,

Here's an updated series, including a runtime abort fix and a GUI bugfix.

Note: Patch 3/4 touches generic code.

Regards,
Andreas

Andreas Färber (4):
  MAINTAINERS: Add Cocoa maintainer
  softfloat: Avoid uint16 type conflict on Darwin
  vl.c: Guard against GThread double-initialization
  cocoa: Close sheet after image file selection

 MAINTAINERS     |    5 +++++
 fpu/softfloat.h |    3 +++
 ui/cocoa.m      |    2 ++
 vl.c            |    4 +++-
 4 files changed, 13 insertions(+), 1 deletions(-)

[Qemu-devel] [PATCH v2 1/4] MAINTAINERS: Add Cocoa maintainer

[Andreas Färber]

Degrade cocoa.m from Maintained to Odd Fixes.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Anthony Liguori <aliguori@us.ibm.com>
---
 MAINTAINERS |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2b4c5d7..c08951c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -446,6 +446,11 @@ M: Anthony Liguori <aliguori@us.ibm.com>
 S: Maintained
 F: ui/
 
+Cocoa graphics
+M: Andreas Färber <andreas.faerber@web.de>
+S: Odd Fixes
+F: ui/cocoa.m
+
 Main loop
 M: Anthony Liguori <aliguori@us.ibm.com>
 S: Supported
-- 
1.7.0.3

[Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

In file included from ./bswap.h:7,
                 from ./qemu-common.h:106,
                 from ./qemu-aio.h:17,
                 from ./Block.h:4,
                 from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/FSEvents.h:28,
                 from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/CarbonCore.h:218,
                 from /System/Library/Frameworks/CoreServices.framework/Frameworks/AE.framework/Headers/AE.h:20,
                 from /System/Library/Frameworks/CoreServices.framework/Headers/CoreServices.h:21,
                 from /System/Library/Frameworks/Foundation.framework/Headers/NSURLError.h:17,
                 from /System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:81,
                 from /System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12,
                 from ui/cocoa.m:25:
/Users/andreas/QEMU/qemu/fpu/softfloat.h:60: error: conflicting types for ‘uint16’
/System/Library/Frameworks/Security.framework/Headers/cssmconfig.h:73: error: previous declaration of ‘uint16’ was here
make: *** [ui/cocoa.o] Error 1

Apple's FSEvents.h has #include <Block.h>, which wants
/usr/include/Block.h but due to case-insensitive file system and
include path jungle gets QEMU's ./block.h, which in turn includes
softfloat.h indirectly.

Therefore work around the conflict in softfloat.h itself
by renaming specifically uint16 on Darwin to qemu_uint16.
This fixes the build until we have a more general solution.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Juan Pineda <juan@logician.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 fpu/softfloat.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 07c2929..5320945 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -54,6 +54,9 @@ these four paragraphs for those parts of this code that are retained.
 | to the same as `int'.
 *----------------------------------------------------------------------------*/
 typedef uint8_t flag;
+#ifdef __APPLE__
+#define uint16 qemu_uint16
+#endif
 typedef uint8_t uint8;
 typedef int8_t int8;
 #ifndef _AIX
-- 
1.7.0.3

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Peter Maydell]

On 31 October 2011 19:18, Andreas Färber <andreas.faerber@web.de> wrote:
> --- a/fpu/softfloat.h
> +++ b/fpu/softfloat.h
> @@ -54,6 +54,9 @@ these four paragraphs for those parts of this code that are retained.
>  | to the same as `int'.
>  *----------------------------------------------------------------------------*/
>  typedef uint8_t flag;
> +#ifdef __APPLE__
> +#define uint16 qemu_uint16
> +#endif
>  typedef uint8_t uint8;
>  typedef int8_t int8;
>  #ifndef _AIX

I hate this, but I don't think I quite hate it enough to leave compilation
on Macs broken in 1.0...

Ideally we should just make a decision about whether we want option
(1) or (2) from
http://permalink.gmane.org/gmane.comp.emulators.qemu/115848

-- PMM

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 31.10.2011 20:42, schrieb Peter Maydell:
> On 31 October 2011 19:18, Andreas Färber <andreas.faerber@web.de> wrote:
>> --- a/fpu/softfloat.h
>> +++ b/fpu/softfloat.h
>> @@ -54,6 +54,9 @@ these four paragraphs for those parts of this code that are retained.
>>  | to the same as `int'.
>>  *----------------------------------------------------------------------------*/
>>  typedef uint8_t flag;
>> +#ifdef __APPLE__
>> +#define uint16 qemu_uint16
>> +#endif
>>  typedef uint8_t uint8;
>>  typedef int8_t int8;
>>  #ifndef _AIX
> 
> I hate this, but I don't think I quite hate it enough to leave compilation
> on Macs broken in 1.0...
> 
> Ideally we should just make a decision about whether we want option
> (1) or (2) from
> http://permalink.gmane.org/gmane.comp.emulators.qemu/115848

That's what "more general solution" refers to.

At this point in time though I'd much prefer to commit a non-intrusive
three-line quickfix (which we can easily remove later on) than no
solution at all or than to refactor lots of softfloat code.

Andreas

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Eric Sunshine]

On Oct 31, 2011, at 3:18 PM, Andreas Färber wrote:
> In file included from ./bswap.h:7,
>                 from ./qemu-common.h:106,
>                 from ./qemu-aio.h:17,
>                 from ./Block.h:4,
>                 from /System/Library/Frameworks/ 
> CoreServices.framework/Frameworks/CarbonCore.framework/Headers/ 
> FSEvents.h:28,
>                 from /System/Library/Frameworks/ 
> CoreServices.framework/Frameworks/CarbonCore.framework/Headers/ 
> CarbonCore.h:218,
>                 from /System/Library/Frameworks/ 
> CoreServices.framework/Frameworks/AE.framework/Headers/AE.h:20,
>                 from /System/Library/Frameworks/ 
> CoreServices.framework/Headers/CoreServices.h:21,
>                 from /System/Library/Frameworks/Foundation.framework/ 
> Headers/NSURLError.h:17,
>                 from /System/Library/Frameworks/Foundation.framework/ 
> Headers/Foundation.h:81,
>                 from /System/Library/Frameworks/Cocoa.framework/ 
> Headers/Cocoa.h:12,
>                 from ui/cocoa.m:25:
> /Users/andreas/QEMU/qemu/fpu/softfloat.h:60: error: conflicting  
> types for ‘uint16’
> /System/Library/Frameworks/Security.framework/Headers/cssmconfig.h: 
> 73: error: previous declaration of ‘uint16’ was here
> make: *** [ui/cocoa.o] Error 1
>
> Apple's FSEvents.h has #include <Block.h>, which wants
> /usr/include/Block.h but due to case-insensitive file system and
> include path jungle gets QEMU's ./block.h, which in turn includes
> softfloat.h indirectly.
>
> Therefore work around the conflict in softfloat.h itself
> by renaming specifically uint16 on Darwin to qemu_uint16.
> This fixes the build until we have a more general solution.
>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> Cc: Juan Pineda <juan@logician.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> ---
> fpu/softfloat.h |    3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/fpu/softfloat.h b/fpu/softfloat.h
> index 07c2929..5320945 100644
> --- a/fpu/softfloat.h
> +++ b/fpu/softfloat.h
> @@ -54,6 +54,9 @@ these four paragraphs for those parts of this code  
> that are retained.
> | to the same as `int'.
> *----------------------------------------------------------------------------*/
> typedef uint8_t flag;
> +#ifdef __APPLE__
> +#define uint16 qemu_uint16
> +#endif
> typedef uint8_t uint8;
> typedef int8_t int8;
> #ifndef _AIX

Perhaps the following alternative solution would be more palatable?  
It's still tremendously ugly, but is localized to cocoa.m, thus less  
intrusive.

-- >8 --
Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin

cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/Cocoa.h>.
cssmconfig.h defines type uint16 which unfortunately conflicts with the
definition in qemu's softfloat.h, thus resulting in compilation failure.
To work around the problem, #define _UINT16, which informs cssmconfig.h
that uint16 is already defined and that it should not apply its own
definition. Additionally, ensure that <Cocoa/Cocoa.h> is included after
softfloat.h rather than before since some Cocoa headers expect type
uint16 to exist.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
  ui/cocoa.m |    7 ++++---
  1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index d9e4e3d..ac15418 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -22,13 +22,14 @@
   * THE SOFTWARE.
   */

-#import <Cocoa/Cocoa.h>
-#include <crt_externs.h>
-
  #include "qemu-common.h"
  #include "console.h"
  #include "sysemu.h"

+#define _UINT16
+#import <Cocoa/Cocoa.h>
+#include <crt_externs.h>
+
  #ifndef MAC_OS_X_VERSION_10_4
  #define MAC_OS_X_VERSION_10_4 1040
  #endif
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 09:09, schrieb Eric Sunshine:
> Perhaps the following alternative solution would be more palatable? It's
> still tremendously ugly, but is localized to cocoa.m, thus less intrusive.
> 
> -- >8 --
> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
> 
> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/Cocoa.h>.
> cssmconfig.h defines type uint16 which unfortunately conflicts with the
> definition in qemu's softfloat.h, thus resulting in compilation failure.
> To work around the problem, #define _UINT16, which informs cssmconfig.h
> that uint16 is already defined and that it should not apply its own
> definition.

Thanks for the suggestion! _UINT16 is an interesting suggestion, however
softfloat's uint16 is not uint16_t but int, so I'd rather not do it that
way around.

(I had also decided against the AIX path of never defining uint16 and
always using system definitions, since that wouldn't work outside Cocoa
code.)

Do you have any thoughts about the include path issue? If we could keep
QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
redefine the system type instead, in cocoa.m.

Andreas

[Qemu-devel] [PATCH v3 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

On Mac OS X we get:

In file included from ./bswap.h:7,
                 from ./qemu-common.h:106,
                 from ./qemu-aio.h:17,
                 from ./Block.h:4,
                 from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/FSEvents.h:28,
                 from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/CarbonCore.h:218,
                 from /System/Library/Frameworks/CoreServices.framework/Frameworks/AE.framework/Headers/AE.h:20,
                 from /System/Library/Frameworks/CoreServices.framework/Headers/CoreServices.h:21,
                 from /System/Library/Frameworks/Foundation.framework/Headers/NSURLError.h:17,
                 from /System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:81,
                 from /System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12,
                 from ui/cocoa.m:25:
/Users/andreas/QEMU/qemu/fpu/softfloat.h:60: error: conflicting types for ‘uint16’
/System/Library/Frameworks/Security.framework/Headers/cssmconfig.h:73: error: previous declaration of ‘uint16’ was here
make: *** [ui/cocoa.o] Error 1

Apple's FSEvents.h has #include <Block.h>, which wants
/usr/include/Block.h but due to case-insensitive file system and
include path jungle gets QEMU's ./block.h, which in turn includes
softfloat.h indirectly.

Therefore work around the conflict in softfloat.h itself,
by reusing uint16 from Security/cssmconfig.h on Darwin.
This fixes the build until we have a more general solution.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Eric Sunshine <sunshine@sunshineco.com>
Cc: Juan Pineda <juan@logician.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 fpu/softfloat.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 07c2929..229d834 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -57,7 +57,9 @@ typedef uint8_t flag;
 typedef uint8_t uint8;
 typedef int8_t int8;
 #ifndef _AIX
+#if !(defined(__APPLE__) && defined(_UINT16))
 typedef int uint16;
+#endif
 typedef int int16;
 #endif
 typedef unsigned int uint32;
-- 
1.7.0.3

Re: [Qemu-devel] [PATCH v3 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Peter Maydell]

On 1 November 2011 17:59, Andreas Färber <andreas.faerber@web.de> wrote:
> Apple's FSEvents.h has #include <Block.h>, which wants
> /usr/include/Block.h but due to case-insensitive file system and
> include path jungle gets QEMU's ./block.h, which in turn includes
> softfloat.h indirectly.

Incidentally, surely you need to fix this anyway (ie fix the
include path issue somehow)?

-- PMM

Re: [Qemu-devel] [PATCH v3 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 19:01, schrieb Peter Maydell:
> On 1 November 2011 17:59, Andreas Färber <andreas.faerber@web.de> wrote:
>> Apple's FSEvents.h has #include <Block.h>, which wants
>> /usr/include/Block.h but due to case-insensitive file system and
>> include path jungle gets QEMU's ./block.h, which in turn includes
>> softfloat.h indirectly.
> 
> Incidentally, surely you need to fix this anyway (ie fix the
> include path issue somehow)?

Yes, that's why I'm explicitly documenting it. I have no clue how to fix
it though. Experts' opinion welcome!

Andreas

Re: [Qemu-devel] [PATCH v3 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Eric Sunshine]

On Nov 1, 2011, at 2:05 PM, Andreas Färber wrote:
> Am 01.11.2011 19:01, schrieb Peter Maydell:
>> On 1 November 2011 17:59, Andreas Färber <andreas.faerber@web.de>  
>> wrote:
>>> Apple's FSEvents.h has #include <Block.h>, which wants
>>> /usr/include/Block.h but due to case-insensitive file system and
>>> include path jungle gets QEMU's ./block.h, which in turn includes
>>> softfloat.h indirectly.
>>
>> Incidentally, surely you need to fix this anyway (ie fix the
>> include path issue somehow)?
>
> Yes, that's why I'm explicitly documenting it. I have no clue how to  
> fix
> it though. Experts' opinion welcome!


It probably is not a good idea to emphasize the particular #include  
stack issued by this one build environment (presumably XCode on Lion?)  
so loudly in the commit message. The type redefinition error will  
manifest regardless of #include ordering and regardless of whether or  
not ./block.h is picked up accidentally instead of <Block.h>. For  
instance, on Leopard with XCode 3.1.4, the #include stack emitted with  
the error message is entirely different even though the underlying  
issue is the same:

   OBJC  ui/cocoa.o
In file included from ./bswap.h:8,
                  from ./qemu-common.h:107,
                  from ui/cocoa.m:29:
/Users/sunshine/Desktop/qemu/fpu/softfloat.h:60: error: conflicting  
types for 'uint16'
/System/Library/Frameworks/Security.framework/Headers/cssmconfig.h:68:  
error: previous declaration of 'uint16' was here
make: *** [ui/cocoa.o] Error 1

In fact, on Leopard, FSEvents.h does not #include <Block.h>, so  
focusing on it in the log message is misleading.

-- ES

Re: [Qemu-devel] [PATCH v3 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 19:55, schrieb Eric Sunshine:
> On Nov 1, 2011, at 2:05 PM, Andreas Färber wrote:
>> Am 01.11.2011 19:01, schrieb Peter Maydell:
>>> On 1 November 2011 17:59, Andreas Färber <andreas.faerber@web.de> wrote:
>>>> Apple's FSEvents.h has #include <Block.h>, which wants
>>>> /usr/include/Block.h but due to case-insensitive file system and
>>>> include path jungle gets QEMU's ./block.h, which in turn includes
>>>> softfloat.h indirectly.
>>>
>>> Incidentally, surely you need to fix this anyway (ie fix the
>>> include path issue somehow)?
>>
>> Yes, that's why I'm explicitly documenting it. I have no clue how to fix
>> it though. Experts' opinion welcome!
> 
> 
> It probably is not a good idea to emphasize the particular #include
> stack issued by this one build environment (presumably XCode on Lion?)
> so loudly in the commit message. The type redefinition error will
> manifest regardless of #include ordering and regardless of whether or
> not ./block.h is picked up accidentally instead of <Block.h>. For
> instance, on Leopard with XCode 3.1.4, the #include stack emitted with
> the error message is entirely different even though the underlying issue
> is the same:
> 
>   OBJC  ui/cocoa.o
> In file included from ./bswap.h:8,
>                  from ./qemu-common.h:107,
>                  from ui/cocoa.m:29:
> /Users/sunshine/Desktop/qemu/fpu/softfloat.h:60: error: conflicting
> types for 'uint16'
> /System/Library/Frameworks/Security.framework/Headers/cssmconfig.h:68:
> error: previous declaration of 'uint16' was here
> make: *** [ui/cocoa.o] Error 1
> 
> In fact, on Leopard, FSEvents.h does not #include <Block.h>, so focusing
> on it in the log message is misleading.

My point is, that's the reason it needs to be fixed *this* way and not
in cocoa.m like you'd expect. If it doesn't show up in git-blame, people
will not understand and "clean up". It's always just an example since it
includes user-specific paths - here Snow Leopard. Rewording.

AF

Re: [Qemu-devel] [PATCH v3 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 18:59, schrieb Andreas Färber:
> On Mac OS X we get:
> 
> In file included from ./bswap.h:7,
>                  from ./qemu-common.h:106,
>                  from ./qemu-aio.h:17,
>                  from ./Block.h:4,
>                  from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/FSEvents.h:28,
>                  from /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/CarbonCore.h:218,
>                  from /System/Library/Frameworks/CoreServices.framework/Frameworks/AE.framework/Headers/AE.h:20,
>                  from /System/Library/Frameworks/CoreServices.framework/Headers/CoreServices.h:21,
>                  from /System/Library/Frameworks/Foundation.framework/Headers/NSURLError.h:17,
>                  from /System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:81,
>                  from /System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12,
>                  from ui/cocoa.m:25:
> /Users/andreas/QEMU/qemu/fpu/softfloat.h:60: error: conflicting types for ‘uint16’
> /System/Library/Frameworks/Security.framework/Headers/cssmconfig.h:73: error: previous declaration of ‘uint16’ was here
> make: *** [ui/cocoa.o] Error 1
> 
> Apple's FSEvents.h has #include <Block.h>, which wants
> /usr/include/Block.h but due to case-insensitive file system and
> include path jungle gets QEMU's ./block.h, which in turn includes
> softfloat.h indirectly.
> 
> Therefore work around the conflict in softfloat.h itself,
> by reusing uint16 from Security/cssmconfig.h on Darwin.
> This fixes the build until we have a more general solution.
> 
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> Cc: Eric Sunshine <sunshine@sunshineco.com>
> Cc: Juan Pineda <juan@logician.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>

v3 series is available from
git://repo.or.cz/qemu/afaerber.git cocoa-for-upstream

If no further protest arises, I'll send a pull request later today.

Andreas

> ---
>  fpu/softfloat.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/fpu/softfloat.h b/fpu/softfloat.h
> index 07c2929..229d834 100644
> --- a/fpu/softfloat.h
> +++ b/fpu/softfloat.h
> @@ -57,7 +57,9 @@ typedef uint8_t flag;
>  typedef uint8_t uint8;
>  typedef int8_t int8;
>  #ifndef _AIX
> +#if !(defined(__APPLE__) && defined(_UINT16))
>  typedef int uint16;
> +#endif
>  typedef int int16;
>  #endif
>  typedef unsigned int uint32;

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Eric Sunshine]

On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>> Perhaps the following alternative solution would be more palatable?  
>> It's
>> still tremendously ugly, but is localized to cocoa.m, thus less  
>> intrusive.
>>
>> -- >8 --
>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>
>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/ 
>> Cocoa.h>.
>> cssmconfig.h defines type uint16 which unfortunately conflicts with  
>> the
>> definition in qemu's softfloat.h, thus resulting in compilation  
>> failure.
>> To work around the problem, #define _UINT16, which informs  
>> cssmconfig.h
>> that uint16 is already defined and that it should not apply its own
>> definition.
>
> Thanks for the suggestion! _UINT16 is an interesting suggestion,  
> however
> softfloat's uint16 is not uint16_t but int, so I'd rather not do it  
> that
> way around.
>
> (I had also decided against the AIX path of never defining uint16 and
> always using system definitions, since that wouldn't work outside  
> Cocoa
> code.)
>
> Do you have any thoughts about the include path issue? If we could  
> keep
> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
> redefine the system type instead, in cocoa.m.

Is the intention to trust uint16 from <Security/cssmconfig.h> over the  
one softfloat.h? If so, shouldn't we be taking as many type  
definitions from <Security/cssmconfig.h> as we can rather than just  
this one? (I'm not recommending it; just trying to understand the goal.)

-- ES

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 19:47, schrieb Eric Sunshine:
> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
>> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>>> Perhaps the following alternative solution would be more palatable? It's
>>> still tremendously ugly, but is localized to cocoa.m, thus less
>>> intrusive.
>>>
>>> -- >8 --
>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>>
>>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/Cocoa.h>.
>>> cssmconfig.h defines type uint16 which unfortunately conflicts with the
>>> definition in qemu's softfloat.h, thus resulting in compilation failure.
>>> To work around the problem, #define _UINT16, which informs cssmconfig.h
>>> that uint16 is already defined and that it should not apply its own
>>> definition.
>>
>> Thanks for the suggestion! _UINT16 is an interesting suggestion, however
>> softfloat's uint16 is not uint16_t but int, so I'd rather not do it that
>> way around.
>>
>> (I had also decided against the AIX path of never defining uint16 and
>> always using system definitions, since that wouldn't work outside Cocoa
>> code.)
>>
>> Do you have any thoughts about the include path issue? If we could keep
>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
>> redefine the system type instead, in cocoa.m.
> 
> Is the intention to trust uint16 from <Security/cssmconfig.h> over the
> one softfloat.h? If so, shouldn't we be taking as many type definitions
> from <Security/cssmconfig.h> as we can rather than just this one? (I'm
> not recommending it; just trying to understand the goal.)

Short-term goal: make Darwin build 1.0 without breaking others
Long-term goal: not use uint16 etc. in QEMU at all

Don't see what you mean with "taking as many type definitions". After
uint16 I get no further conflicts for --enable-system --disable-user,
so what is there to take?

Andreas

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Eric Sunshine]

On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote:

> Am 01.11.2011 19:47, schrieb Eric Sunshine:
>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
>>> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>>>> Perhaps the following alternative solution would be more  
>>>> palatable? It's
>>>> still tremendously ugly, but is localized to cocoa.m, thus less
>>>> intrusive.
>>>>
>>>> -- >8 --
>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>>>
>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/ 
>>>> Cocoa.h>.
>>>> cssmconfig.h defines type uint16 which unfortunately conflicts  
>>>> with the
>>>> definition in qemu's softfloat.h, thus resulting in compilation  
>>>> failure.
>>>> To work around the problem, #define _UINT16, which informs  
>>>> cssmconfig.h
>>>> that uint16 is already defined and that it should not apply its own
>>>> definition.
>>>
>>> Thanks for the suggestion! _UINT16 is an interesting suggestion,  
>>> however
>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do  
>>> it that
>>> way around.
>>>
>>> (I had also decided against the AIX path of never defining uint16  
>>> and
>>> always using system definitions, since that wouldn't work outside  
>>> Cocoa
>>> code.)
>>>
>>> Do you have any thoughts about the include path issue? If we could  
>>> keep
>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
>>> redefine the system type instead, in cocoa.m.
>>
>> Is the intention to trust uint16 from <Security/cssmconfig.h> over  
>> the
>> one softfloat.h? If so, shouldn't we be taking as many type  
>> definitions
>> from <Security/cssmconfig.h> as we can rather than just this one?  
>> (I'm
>> not recommending it; just trying to understand the goal.)
>
> Short-term goal: make Darwin build 1.0 without breaking others
> Long-term goal: not use uint16 etc. in QEMU at all
>
> Don't see what you mean with "taking as many type definitions". After
> uint16 I get no further conflicts for --enable-system --disable-user,
> so what is there to take?

Sorry for not being clear. My question was not about build errors but  
about semantics. What I meant was that, with this patch, you are  
giving special preference only to Darwin's definition of uint16, but  
then contrarily preferring softfloat's definition of int16. Likewise,  
softfloat's uint32, int32, uint64, int64 from softfloat are trusted  
over the definitions from Darwin.

Other than the fact that only uint16 led to a compilation error, it  
does not make sense semantically to single out Darwin's definition of  
only this one type. I would think that we should be trusting either  
_all_ Darwin type definitions or _none_. Singling out just this one  
seems anomalous.

-- ES

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 20:06, schrieb Eric Sunshine:
> 
> On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote:
> 
>> Am 01.11.2011 19:47, schrieb Eric Sunshine:
>>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
>>>> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>>>>> Perhaps the following alternative solution would be more palatable?
>>>>> It's
>>>>> still tremendously ugly, but is localized to cocoa.m, thus less
>>>>> intrusive.
>>>>>
>>>>> -- >8 --
>>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>>>>
>>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via
>>>>> <Cocoa/Cocoa.h>.
>>>>> cssmconfig.h defines type uint16 which unfortunately conflicts with
>>>>> the
>>>>> definition in qemu's softfloat.h, thus resulting in compilation
>>>>> failure.
>>>>> To work around the problem, #define _UINT16, which informs
>>>>> cssmconfig.h
>>>>> that uint16 is already defined and that it should not apply its own
>>>>> definition.
>>>>
>>>> Thanks for the suggestion! _UINT16 is an interesting suggestion,
>>>> however
>>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do it
>>>> that
>>>> way around.
>>>>
>>>> (I had also decided against the AIX path of never defining uint16 and
>>>> always using system definitions, since that wouldn't work outside Cocoa
>>>> code.)
>>>>
>>>> Do you have any thoughts about the include path issue? If we could keep
>>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
>>>> redefine the system type instead, in cocoa.m.
>>>
>>> Is the intention to trust uint16 from <Security/cssmconfig.h> over the
>>> one softfloat.h? If so, shouldn't we be taking as many type definitions
>>> from <Security/cssmconfig.h> as we can rather than just this one? (I'm
>>> not recommending it; just trying to understand the goal.)
>>
>> Short-term goal: make Darwin build 1.0 without breaking others
>> Long-term goal: not use uint16 etc. in QEMU at all
>>
>> Don't see what you mean with "taking as many type definitions". After
>> uint16 I get no further conflicts for --enable-system --disable-user,
>> so what is there to take?
> 
> Sorry for not being clear. My question was not about build errors but
> about semantics. What I meant was that, with this patch, you are giving
> special preference only to Darwin's definition of uint16, but then
> contrarily preferring softfloat's definition of int16. Likewise,
> softfloat's uint32, int32, uint64, int64 from softfloat are trusted over
> the definitions from Darwin.
> 
> Other than the fact that only uint16 led to a compilation error, it does
> not make sense semantically to single out Darwin's definition of only
> this one type. I would think that we should be trusting either _all_
> Darwin type definitions or _none_. Singling out just this one seems
> anomalous.

Listen, I dont have time for this. We have three options:

1) I can say, "I'm the Cocoa maintainer for multiple years now, I don't
care if someone pops up day before the deadline and complains" and just
push my version of preference.

2) We disagree on the solution, so I'm fair and send a pull request for
the three other non-controversial patches only and 1.0 remains broken on
Darwin.

3) You send a patch based on this one, detailing what additional changes
you suggest and we'll see clearer what exactly you mean.

I'm not preferring any definition of int16, uint32, etc., there simply
is no conflict, so why would I clutter softfloat.h with unnecessary
workarounds that we want to go away anyway.

Feel free to refactor fpu/* instead to not use uint16 in the first
place. I did so once and it was rejected, so I'm not too inclined to do
that again unless we decide on how exactly to proceed with that!

Andreas

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Peter Maydell]

On 1 November 2011 19:25, Andreas Färber <andreas.faerber@web.de> wrote:
> Feel free to refactor fpu/* instead to not use uint16 in the first
> place. I did so once and it was rejected, so I'm not too inclined to do
> that again unless we decide on how exactly to proceed with that!

I think we could probably resolve that with a little bit of benchmarking
about whether it really makes any difference whether we use an int16_t
or an int32_t for the variables where softfloat has int16 currently;
but benchmarking is tedious so I've never got round to it :-)

-- PMM

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Eric Sunshine]

On Nov 1, 2011, at 3:25 PM, Andreas Färber wrote:
> Am 01.11.2011 20:06, schrieb Eric Sunshine:
>>
>> On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote:
>>
>>> Am 01.11.2011 19:47, schrieb Eric Sunshine:
>>>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
>>>>> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>>>>>> Perhaps the following alternative solution would be more  
>>>>>> palatable?
>>>>>> It's
>>>>>> still tremendously ugly, but is localized to cocoa.m, thus less
>>>>>> intrusive.
>>>>>>
>>>>>> -- >8 --
>>>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>>>>>
>>>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via
>>>>>> <Cocoa/Cocoa.h>.
>>>>>> cssmconfig.h defines type uint16 which unfortunately conflicts  
>>>>>> with
>>>>>> the
>>>>>> definition in qemu's softfloat.h, thus resulting in compilation
>>>>>> failure.
>>>>>> To work around the problem, #define _UINT16, which informs
>>>>>> cssmconfig.h
>>>>>> that uint16 is already defined and that it should not apply its  
>>>>>> own
>>>>>> definition.
>>>>>
>>>>> Thanks for the suggestion! _UINT16 is an interesting suggestion,
>>>>> however
>>>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do  
>>>>> it
>>>>> that
>>>>> way around.
>>>>>
>>>>> (I had also decided against the AIX path of never defining  
>>>>> uint16 and
>>>>> always using system definitions, since that wouldn't work  
>>>>> outside Cocoa
>>>>> code.)
>>>>>
>>>>> Do you have any thoughts about the include path issue? If we  
>>>>> could keep
>>>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
>>>>> redefine the system type instead, in cocoa.m.
>>>>
>>>> Is the intention to trust uint16 from <Security/cssmconfig.h>  
>>>> over the
>>>> one softfloat.h? If so, shouldn't we be taking as many type  
>>>> definitions
>>>> from <Security/cssmconfig.h> as we can rather than just this one?  
>>>> (I'm
>>>> not recommending it; just trying to understand the goal.)
>>>
>>> Short-term goal: make Darwin build 1.0 without breaking others
>>> Long-term goal: not use uint16 etc. in QEMU at all
>>>
>>> Don't see what you mean with "taking as many type definitions".  
>>> After
>>> uint16 I get no further conflicts for --enable-system --disable- 
>>> user,
>>> so what is there to take?
>>
>> Sorry for not being clear. My question was not about build errors but
>> about semantics. What I meant was that, with this patch, you are  
>> giving
>> special preference only to Darwin's definition of uint16, but then
>> contrarily preferring softfloat's definition of int16. Likewise,
>> softfloat's uint32, int32, uint64, int64 from softfloat are trusted  
>> over
>> the definitions from Darwin.
>>
>> Other than the fact that only uint16 led to a compilation error, it  
>> does
>> not make sense semantically to single out Darwin's definition of only
>> this one type. I would think that we should be trusting either _all_
>> Darwin type definitions or _none_. Singling out just this one seems
>> anomalous.
>
> Listen, I dont have time for this. We have three options:
>
> 1) I can say, "I'm the Cocoa maintainer for multiple years now, I  
> don't
> care if someone pops up day before the deadline and complains" and  
> just
> push my version of preference.

I hope that you do not interpret my alternate patch as a "complaint  
before the deadline". My intention only was to be helpful when I saw  
Peter's response [1], and thought that a less intrusive patch might be  
more acceptable.

[1]: http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg03936.html

-- ES

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 20:45, schrieb Eric Sunshine:
> On Nov 1, 2011, at 3:25 PM, Andreas Färber wrote:
>> Am 01.11.2011 20:06, schrieb Eric Sunshine:
>>>
>>> On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote:
>>>
>>>> Am 01.11.2011 19:47, schrieb Eric Sunshine:
>>>>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
>>>>>> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>>>>>>> Perhaps the following alternative solution would be more palatable?
>>>>>>> It's
>>>>>>> still tremendously ugly, but is localized to cocoa.m, thus less
>>>>>>> intrusive.
>>>>>>>
>>>>>>> -- >8 --
>>>>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>>>>>>
>>>>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via
>>>>>>> <Cocoa/Cocoa.h>.
>>>>>>> cssmconfig.h defines type uint16 which unfortunately conflicts with
>>>>>>> the
>>>>>>> definition in qemu's softfloat.h, thus resulting in compilation
>>>>>>> failure.
>>>>>>> To work around the problem, #define _UINT16, which informs
>>>>>>> cssmconfig.h
>>>>>>> that uint16 is already defined and that it should not apply its own
>>>>>>> definition.
>>>>>>
>>>>>> Thanks for the suggestion! _UINT16 is an interesting suggestion,
>>>>>> however
>>>>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do it
>>>>>> that
>>>>>> way around.
>>>>>>
>>>>>> (I had also decided against the AIX path of never defining uint16 and
>>>>>> always using system definitions, since that wouldn't work outside
>>>>>> Cocoa
>>>>>> code.)
>>>>>>
>>>>>> Do you have any thoughts about the include path issue? If we could
>>>>>> keep
>>>>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
>>>>>> redefine the system type instead, in cocoa.m.
>>>>>
>>>>> Is the intention to trust uint16 from <Security/cssmconfig.h> over the
>>>>> one softfloat.h? If so, shouldn't we be taking as many type
>>>>> definitions
>>>>> from <Security/cssmconfig.h> as we can rather than just this one? (I'm
>>>>> not recommending it; just trying to understand the goal.)
>>>>
>>>> Short-term goal: make Darwin build 1.0 without breaking others
>>>> Long-term goal: not use uint16 etc. in QEMU at all
>>>>
>>>> Don't see what you mean with "taking as many type definitions". After
>>>> uint16 I get no further conflicts for --enable-system --disable-user,
>>>> so what is there to take?
>>>
>>> Sorry for not being clear. My question was not about build errors but
>>> about semantics. What I meant was that, with this patch, you are giving
>>> special preference only to Darwin's definition of uint16, but then
>>> contrarily preferring softfloat's definition of int16. Likewise,
>>> softfloat's uint32, int32, uint64, int64 from softfloat are trusted over
>>> the definitions from Darwin.
>>>
>>> Other than the fact that only uint16 led to a compilation error, it does
>>> not make sense semantically to single out Darwin's definition of only
>>> this one type. I would think that we should be trusting either _all_
>>> Darwin type definitions or _none_. Singling out just this one seems
>>> anomalous.
>>
>> Listen, I dont have time for this. We have three options:
>>
>> 1) I can say, "I'm the Cocoa maintainer for multiple years now, I don't
>> care if someone pops up day before the deadline and complains" and just
>> push my version of preference.
> 
> I hope that you do not interpret my alternate patch as a "complaint
> before the deadline".

Not your patch, I thanked you for it, but the seemingly nonconstructive
complaints about my follow-up. I would've much preferred code. :/

> My intention only was to be helpful when I saw
> Peter's response [1], and thought that a less intrusive patch might be
> more acceptable.
> 
> [1]: http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg03936.html

Then it's not the intention we differ in, I had tried some solutions
inside ui/cocoa.m myself before.

Apart from non-intrusiveness further criteria are reversibility of the
short-term fix[1] and ABI safety.

I'll happily review new patches from tomorrow on.

Regards,
Andreas

[1] We have similar lurking issues with [u]int* on Haiku/BeOS.

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Eric Sunshine]

On Nov 1, 2011, at 3:06 PM, Eric Sunshine wrote:
> On Nov 1, 2011, at 2:52 PM, Andreas Färber wrote:
>
>> Am 01.11.2011 19:47, schrieb Eric Sunshine:
>>> On Nov 1, 2011, at 12:37 PM, Andreas Färber wrote:
>>>> Am 01.11.2011 09:09, schrieb Eric Sunshine:
>>>>> Perhaps the following alternative solution would be more  
>>>>> palatable? It's
>>>>> still tremendously ugly, but is localized to cocoa.m, thus less
>>>>> intrusive.
>>>>>
>>>>> -- >8 --
>>>>> Subject: [PATCH] softfloat: Avoid uint16 type conflict on Darwin
>>>>>
>>>>> cocoa.m includes <Security/cssmconfig.h> indirectly via <Cocoa/ 
>>>>> Cocoa.h>.
>>>>> cssmconfig.h defines type uint16 which unfortunately conflicts  
>>>>> with the
>>>>> definition in qemu's softfloat.h, thus resulting in compilation  
>>>>> failure.
>>>>> To work around the problem, #define _UINT16, which informs  
>>>>> cssmconfig.h
>>>>> that uint16 is already defined and that it should not apply its  
>>>>> own
>>>>> definition.
>>>>
>>>> Thanks for the suggestion! _UINT16 is an interesting suggestion,  
>>>> however
>>>> softfloat's uint16 is not uint16_t but int, so I'd rather not do  
>>>> it that
>>>> way around.
>>>>
>>>> (I had also decided against the AIX path of never defining uint16  
>>>> and
>>>> always using system definitions, since that wouldn't work outside  
>>>> Cocoa
>>>> code.)
>>>>
>>>> Do you have any thoughts about the include path issue? If we  
>>>> could keep
>>>> QEMU code from getting into #import <Cocoa/Cocoa.h> then we could
>>>> redefine the system type instead, in cocoa.m.
>>>
>>> Is the intention to trust uint16 from <Security/cssmconfig.h> over  
>>> the
>>> one softfloat.h? If so, shouldn't we be taking as many type  
>>> definitions
>>> from <Security/cssmconfig.h> as we can rather than just this one?  
>>> (I'm
>>> not recommending it; just trying to understand the goal.)
>>
>> Short-term goal: make Darwin build 1.0 without breaking others
>> Long-term goal: not use uint16 etc. in QEMU at all
>>
>> Don't see what you mean with "taking as many type definitions". After
>> uint16 I get no further conflicts for --enable-system --disable-user,
>> so what is there to take?
>
> Sorry for not being clear. My question was not about build errors  
> but about semantics. What I meant was that, with this patch, you are  
> giving special preference only to Darwin's definition of uint16, but  
> then contrarily preferring softfloat's definition of int16.  
> Likewise, softfloat's uint32, int32, uint64, int64 from softfloat  
> are trusted over the definitions from Darwin.
>
> Other than the fact that only uint16 led to a compilation error, it  
> does not make sense semantically to single out Darwin's definition  
> of only this one type. I would think that we should be trusting  
> either _all_ Darwin type definitions or _none_. Singling out just  
> this one seems anomalous.
>
> -- ES

I forgot to mention that with your patch, only cocoa.m is seeing  
Darwin's definition of uint16. The rest of qemu is seeing the  
definition from softfloat.h. This inconsistency hopefully is not  
harmful in the short-term, which is why I asked about the goal. If the  
short-term idea is for cocoa.m to build cleanly but not to worry much  
that cocoa.m sees a different uint16 from the rest of qemu, then the  
less intrusive patch involving only cocoa.m may be preferable.

-- ES

Re: [Qemu-devel] [PATCH v2 2/4] softfloat: Avoid uint16 type conflict on Darwin

[Andreas Färber]

Am 01.11.2011 20:25, schrieb Eric Sunshine:
> I forgot to mention that with your patch, only cocoa.m is seeing
> Darwin's definition of uint16. The rest of qemu is seeing the definition
> from softfloat.h. This inconsistency hopefully is not harmful in the
> short-term, which is why I asked about the goal. If the short-term idea
> is for cocoa.m to build cleanly but not to worry much that cocoa.m sees
> a different uint16 from the rest of qemu, then the less intrusive patch
> involving only cocoa.m may be preferable.

Ouch. Meaning both our softfloat patches are wrong, so I'll go with (2).
It's evening in Italy and the last day for submaintainer PULLs.

AF

[Qemu-devel] [PATCH v2 3/4] vl.c: Guard against GThread double-initialization

[Andreas Färber]

GThread-ERROR **: GThread system may only be initialized once.
aborting...

Making the g_thread_init() call in vl.c conditional resolves an abort on
Mac OS X, where coroutine-gthread.c seems to call it before vl.c.

Reported-by: Juan Pineda <juan@logician.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Daniel P. Berrange <berrange@redhat.com>
---
 vl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 1ddb17b..46b2909 100644
--- a/vl.c
+++ b/vl.c
@@ -2185,7 +2185,9 @@ int main(int argc, char **argv, char **envp)
     error_set_progname(argv[0]);
 
     g_mem_set_vtable(&mem_trace);
-    g_thread_init(NULL);
+    if (!g_thread_supported()) {
+        g_thread_init(NULL);
+    }
 
     runstate_init();
 
-- 
1.7.0.3

[Qemu-devel] [PATCH v2 4/4] cocoa: Close sheet after image file selection

[Andreas Färber]

If no disk image is specified, the Cocoa frontend displays a modal sheet
to let the user select an image file to boot from.

This sheet is never closed and it permanently obscures the emulator window.

Close it after obtaining the file name in case the user did select a file.
Otherwise we exit immediately, so no need to close then.

Signed-off-by: Juan Pineda <juan@logician.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 ui/cocoa.m |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index d9e4e3d..0711205 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -811,6 +811,8 @@ QemuCocoaView *cocoaView;
 
         char **argv = (char**)malloc( sizeof(char*)*3 );
 
+        [sheet close];
+
         asprintf(&argv[0], "%s", bin);
         asprintf(&argv[1], "-hda");
         asprintf(&argv[2], "%s", img);
-- 
1.7.0.3