[Qemu-devel] [PATCH 0/6] Fix compilation issues under darwin

[Qemu-devel] [PATCH 0/6] Fix compilation issues under darwin

[Alexandre Raymond]

Hello everyone,

The following series contains trivial patches to fix several minor issues
encountered while compiling qemu under OSX 10.6.7.

I used [./configure --disable-bsd-user --disable-darwin-user --enable-io-thread]
to configure the build.

Cheers,
Alexandre

Alexandre Raymond (6):
  Fix incorrect check for fdatasync() in configure
  Cocoa: avoid displaying window when command-line contains '-h'
  Fix compilation warning due to incorrectly specified type
  Fix missing prototype under cocoa for qemu_main
  Remove warning in printf due to type mismatch
  Avoid compilation warning regarding kvm under darwin

 audio/coreaudio.c       |    2 +-
 configure               |    8 +++++++-
 target-lm32/translate.c |    2 +-
 target-s390x/helper.c   |    1 -
 ui/cocoa.m              |    3 ++-
 vl.c                    |    1 +
 6 files changed, 12 insertions(+), 5 deletions(-)

-- 
1.7.5

[Qemu-devel] [PATCH 1/6] Fix incorrect check for fdatasync() in configure

[Alexandre Raymond]

For some reason, darwin provides a symbol for fdatasync(), but
doesn't officially support it.

The manpage for fdatasync on Linux states the following:

"On POSIX  systems  on  which fdatasync() is available,
_POSIX_SYNCHRONIZED_IO is defined in <unistd.h> to a value greater than 0."

In fact, unistd.h defines this value to "-1", at least on OSX 10.6.7.

Add this check to the configure file.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 configure |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index a318d37..b21ef75 100755
--- a/configure
+++ b/configure
@@ -2477,7 +2477,13 @@ fi
 fdatasync=no
 cat > $TMPC << EOF
 #include <unistd.h>
-int main(void) { return fdatasync(0); }
+int main(void) {
+#if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
+return fdatasync(0);
+#else
+#abort Not supported
+#endif
+}
 EOF
 if compile_prog "" "" ; then
     fdatasync=yes
-- 
1.7.5

Re: [Qemu-devel] [PATCH 1/6] Fix incorrect check for fdatasync() in configure

[Andreas Färber]

Am 27.05.2011 um 19:22 schrieb Alexandre Raymond:

> For some reason, darwin provides a symbol for fdatasync(), but
> doesn't officially support it.
>
> The manpage for fdatasync on Linux states the following:
>
> "On POSIX  systems  on  which fdatasync() is available,
> _POSIX_SYNCHRONIZED_IO is defined in <unistd.h> to a value greater  
> than 0."

The Open Group Base Specification Issue 7 says this on fdatasync():

"The functionality shall be equivalent to fsync() with the symbol  
_POSIX_SYNCHRONIZED_IO defined, with the exception that all I/O  
operations shall be completed as defined for synchronized I/O data  
integrity completion."

On unistd.h it goes on to say:

"_POSIX_SYNCHRONIZED_IO
[SIO]
The implementation supports the Synchronized Input and Output option.  
If this symbol is defined in <unistd.h>, it shall be defined to be -1,  
0, or 200809L."

The change history has nothing on that define and its value -1, so I'm  
not convinced that this really is The Right Way to check.

> In fact, unistd.h defines this value to "-1", at least on OSX 10.6.7.
>
> Add this check to the configure file.
>
> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>

Andreas

> ---
> configure |    8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index a318d37..b21ef75 100755
> --- a/configure
> +++ b/configure
> @@ -2477,7 +2477,13 @@ fi
> fdatasync=no
> cat > $TMPC << EOF
> #include <unistd.h>
> -int main(void) { return fdatasync(0); }
> +int main(void) {
> +#if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
> +return fdatasync(0);
> +#else
> +#abort Not supported
> +#endif
> +}
> EOF
> if compile_prog "" "" ; then
>     fdatasync=yes
> -- 
> 1.7.5

Re: [Qemu-devel] [PATCH 1/6] Fix incorrect check for fdatasync() in configure

[Alexandre Raymond]

Hi Andreas,

According to this excerpt from The Open Group Base Specifications
Issue 6 (http://pubs.opengroup.org/onlinepubs/009695399/basedefs/unistd.h.html),
'>0' indeed means that the functionality is implemented and can be
used.
----8<----
The following symbolic constants, if defined in <unistd.h>, shall have
a value of -1, 0, or greater, unless otherwise specified below. If
these are undefined, the fpathconf(), pathconf(), or sysconf()
functions can be used to determine whether the option is provided for
a particular invocation of the application.

If a symbolic constant is defined with the value -1, the option is not
supported. Headers, data types, and function interfaces required only
for the option need not be supplied. An application that attempts to
use anything associated only with the option is considered to be
requiring an extension.

If a symbolic constant is defined with a value greater than zero, the
option shall always be supported when the application is executed. All
headers, data types, and functions shall be present and shall operate
as specified.

If a symbolic constant is defined with the value zero, all headers,
data types, and functions shall be present. The application can check
at runtime to see whether the option is supported by calling
fpathconf(), pathconf(), or sysconf() with the indicated name
parameter.
----8<----

Alexandre

On Sun, May 29, 2011 at 10:50 AM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 27.05.2011 um 19:22 schrieb Alexandre Raymond:
>
>> For some reason, darwin provides a symbol for fdatasync(), but
>> doesn't officially support it.
>>
>> The manpage for fdatasync on Linux states the following:
>>
>> "On POSIX  systems  on  which fdatasync() is available,
>> _POSIX_SYNCHRONIZED_IO is defined in <unistd.h> to a value greater than
>> 0."
>
> The Open Group Base Specification Issue 7 says this on fdatasync():
>
> "The functionality shall be equivalent to fsync() with the symbol
> _POSIX_SYNCHRONIZED_IO defined, with the exception that all I/O operations
> shall be completed as defined for synchronized I/O data integrity
> completion."
>
> On unistd.h it goes on to say:
>
> "_POSIX_SYNCHRONIZED_IO
> [SIO]
> The implementation supports the Synchronized Input and Output option. If
> this symbol is defined in <unistd.h>, it shall be defined to be -1, 0, or
> 200809L."
>
> The change history has nothing on that define and its value -1, so I'm not
> convinced that this really is The Right Way to check.
>
>> In fact, unistd.h defines this value to "-1", at least on OSX 10.6.7.
>>
>> Add this check to the configure file.
>>
>> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
>
> Andreas
>
>> ---
>> configure |    8 +++++++-
>> 1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index a318d37..b21ef75 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2477,7 +2477,13 @@ fi
>> fdatasync=no
>> cat > $TMPC << EOF
>> #include <unistd.h>
>> -int main(void) { return fdatasync(0); }
>> +int main(void) {
>> +#if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
>> +return fdatasync(0);
>> +#else
>> +#abort Not supported
>> +#endif
>> +}
>> EOF
>> if compile_prog "" "" ; then
>>    fdatasync=yes
>> --
>> 1.7.5
>
>

Re: [Qemu-devel] [PATCH 1/6] Fix incorrect check for fdatasync() in configure

[Andreas Färber]

Hi Alexandre,

Am 29.05.2011 um 17:46 schrieb Alexandre Raymond:

> According to this excerpt from The Open Group Base Specifications
> Issue 6 (http://pubs.opengroup.org/onlinepubs/009695399/basedefs/unistd.h.html 
> ),
> '>0' indeed means that the functionality is implemented and can be
> used.
> ----8<----
> The following symbolic constants, if defined in <unistd.h>, shall have
> a value of -1, 0, or greater, unless otherwise specified below. If
> these are undefined, the fpathconf(), pathconf(), or sysconf()
> functions can be used to determine whether the option is provided for
> a particular invocation of the application.
>
> If a symbolic constant is defined with the value -1, the option is not
> supported. Headers, data types, and function interfaces required only
> for the option need not be supplied. An application that attempts to
> use anything associated only with the option is considered to be
> requiring an extension.
>
> If a symbolic constant is defined with a value greater than zero, the
> option shall always be supported when the application is executed. All
> headers, data types, and functions shall be present and shall operate
> as specified.
>
> If a symbolic constant is defined with the value zero, all headers,
> data types, and functions shall be present. The application can check
> at runtime to see whether the option is supported by calling
> fpathconf(), pathconf(), or sysconf() with the indicated name
> parameter.
> ----8<----

Indeed, confirmed in Issue 7, thanks a lot. Could you please provide  
me with an updated commit message saying so? Then I'll apply it.

Andreas

[Qemu-devel] [PATCH 2/6] Cocoa: avoid displaying window when command-line contains '-h'

[Alexandre Raymond]

There was already a check in place to avoid displaying a window
in certain modes such as vnc, nographic or curses.

Add a check for '-h' to avoid displaying a window for a split-
second before showing the usage information.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 ui/cocoa.m |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 20f91bc..7fb8d96 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -865,7 +865,8 @@ int main (int argc, const char * argv[]) {
 
     /* In case we don't need to display a window, let's not do that */
     for (i = 1; i < argc; i++) {
-        if (!strcmp(argv[i], "-vnc") ||
+        if (!strcmp(argv[i], "-h") ||
+            !strcmp(argv[i], "-vnc") ||
             !strcmp(argv[i], "-nographic") ||
             !strcmp(argv[i], "-curses")) {
                 return qemu_main(gArgc, gArgv);
-- 
1.7.5

Re: [Qemu-devel] [PATCH 2/6] Cocoa: avoid displaying window when command-line contains '-h'

[Andreas Färber]

Am 27.05.2011 um 19:22 schrieb Alexandre Raymond:

> There was already a check in place to avoid displaying a window
> in certain modes such as vnc, nographic or curses.
>
> Add a check for '-h' to avoid displaying a window for a split-
> second before showing the usage information.
>
> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
> ---
> ui/cocoa.m |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 20f91bc..7fb8d96 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -865,7 +865,8 @@ int main (int argc, const char * argv[]) {
>
>     /* In case we don't need to display a window, let's not do that */
>     for (i = 1; i < argc; i++) {
> -        if (!strcmp(argv[i], "-vnc") ||
> +        if (!strcmp(argv[i], "-h") ||
> +            !strcmp(argv[i], "-vnc") ||
>             !strcmp(argv[i], "-nographic") ||
>             !strcmp(argv[i], "-curses")) {
>                 return qemu_main(gArgc, gArgv);

Nack. I have a patch from Tristan queued (sorry, forgot to send pull  
request before my vacation), please rebase against that.

Cf. http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/cocoa-for-upstream

Also, your patch fails to detect -help.

BTW -M ? and others face a similar detection issue, so I believe while  
adding to this list is a handy short-term fix, we need a better  
solution there. Suggestions and patches welcome.

Andreas

[Qemu-devel] [PATCH 3/6] Fix compilation warning due to incorrectly specified type

[Alexandre Raymond]

In audio/coreaudio.c, a variable named "str" was assigned "const char" values,
which resulted in the following warnings:

-----8<-----
audio/coreaudio.c: In function ‘coreaudio_logstatus’:
audio/coreaudio.c:59: warning: initialization discards qualifiers from pointer target type
audio/coreaudio.c:63: warning: assignment discards qualifiers from pointer target type
(...)
-----8<-----

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 audio/coreaudio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 0a26413..3bd75cd 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -56,7 +56,7 @@ typedef struct coreaudioVoiceOut {
 
 static void coreaudio_logstatus (OSStatus status)
 {
-    char *str = "BUG";
+    const char *str = "BUG";
 
     switch(status) {
     case kAudioHardwareNoError:
-- 
1.7.5

Re: [Qemu-devel] [PATCH 3/6] Fix compilation warning due to incorrectly specified type

[Stefan Weil]

Am 27.05.2011 19:22, schrieb Alexandre Raymond:
> In audio/coreaudio.c, a variable named "str" was assigned "const char" values,
> which resulted in the following warnings:
>
> -----8<-----
> audio/coreaudio.c: In function ‘coreaudio_logstatus’:
> audio/coreaudio.c:59: warning: initialization discards qualifiers from pointer target type
> audio/coreaudio.c:63: warning: assignment discards qualifiers from pointer target type
> (...)
> -----8<-----
>
> Signed-off-by: Alexandre Raymond<cerbere@gmail.com>
> ---
>   audio/coreaudio.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index 0a26413..3bd75cd 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -56,7 +56,7 @@ typedef struct coreaudioVoiceOut {
>
>   static void coreaudio_logstatus (OSStatus status)
>   {
> -    char *str = "BUG";
> +    const char *str = "BUG";
>
>       switch(status) {
>       case kAudioHardwareNoError:
>    

Acked-by: Stefan Weil <weil@mail.berlios.de>

Re: [Qemu-devel] [PATCH 3/6] Fix compilation warning due to incorrectly specified type

[Andreas Färber]

Am 27.05.2011 um 19:42 schrieb Stefan Weil:

> Am 27.05.2011 19:22, schrieb Alexandre Raymond:
>> In audio/coreaudio.c, a variable named "str" was assigned "const  
>> char" values,
>> which resulted in the following warnings:
>>
>> -----8<-----
>> audio/coreaudio.c: In function ‘coreaudio_logstatus’:
>> audio/coreaudio.c:59: warning: initialization discards qualifiers  
>> from pointer target type
>> audio/coreaudio.c:63: warning: assignment discards qualifiers from  
>> pointer target type
>> (...)
>> -----8<-----
>>
>> Signed-off-by: Alexandre Raymond<cerbere@gmail.com>
>> ---
>>  audio/coreaudio.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
>> index 0a26413..3bd75cd 100644
>> --- a/audio/coreaudio.c
>> +++ b/audio/coreaudio.c
>> @@ -56,7 +56,7 @@ typedef struct coreaudioVoiceOut {
>>
>>  static void coreaudio_logstatus (OSStatus status)
>>  {
>> -    char *str = "BUG";
>> +    const char *str = "BUG";
>>
>>      switch(status) {
>>      case kAudioHardwareNoError:
>>
>
> Acked-by: Stefan Weil <weil@mail.berlios.de>

Thanks, applied to the cocoa branch.

Andreas

[Qemu-devel] [PATCH 4/6] Fix missing prototype under cocoa for qemu_main

[Alexandre Raymond]

The following error message was encountered when compiling
with cocoa support because qemu_main did not have a prototype.

-----8<-----
qemu/vl.c:2037: warning: no previous prototype for ‘qemu_main’
-----8<-----

Add its prototype in the COCOA ifdef, similar to what is done for SDL.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 vl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index b362871..b983646 100644
--- a/vl.c
+++ b/vl.c
@@ -107,6 +107,7 @@ int main(int argc, char **argv)
 #endif /* CONFIG_SDL */
 
 #ifdef CONFIG_COCOA
+int qemu_main(int argc, char **argv, char **envp);
 #undef main
 #define main qemu_main
 #endif /* CONFIG_COCOA */
-- 
1.7.5

Re: [Qemu-devel] [PATCH 4/6] Fix missing prototype under cocoa for qemu_main

[Andreas Färber]

Am 27.05.2011 um 19:22 schrieb Alexandre Raymond:

> The following error message was encountered when compiling
> with cocoa support because qemu_main did not have a prototype.
>
> -----8<-----
> qemu/vl.c:2037: warning: no previous prototype for ‘qemu_main’
> -----8<-----
>
> Add its prototype in the COCOA ifdef, similar to what is done for SDL.
>
> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
> ---
> vl.c |    1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index b362871..b983646 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -107,6 +107,7 @@ int main(int argc, char **argv)
> #endif /* CONFIG_SDL */
>
> #ifdef CONFIG_COCOA
> +int qemu_main(int argc, char **argv, char **envp);
> #undef main
> #define main qemu_main
> #endif /* CONFIG_COCOA */

We've seen at least two different ways of fixing this.

I believe my last one was forgotten, waiting for someone to test with  
SDL. The idea was to have the prototype in a header shared between  
vl.c and cocoa.m, so that we don't have to duplicate the prototype.  
I'll see if I can dig it out or will post a new patch.

Andreas

[Qemu-devel] [PATCH] cocoa: Provide central qemu_main() prototype

[Andreas Färber]

This fixes a missing prototype warning in vl.c and obsoletes
the prototype in cocoa.m. Adjust callers in cocoa.m to supply
third argument, which is currently only used on Linux/ppc.

The prototype is designed so that it could be shared with SDL
and other frontends, if desired.

Cc: Alexandre Raymond <cerbere@gmail.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 qemu-common.h |    5 +++++
 ui/cocoa.m    |    6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/qemu-common.h b/qemu-common.h
index b851b20..218289c 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -131,6 +131,11 @@ static inline char *realpath(const char *path, char *resolved_path)
 
 #endif /* !defined(NEED_CPU_H) */
 
+/* main function, renamed */
+#if defined(CONFIG_COCOA)
+int qemu_main(int argc, char **argv, char **envp);
+#endif
+
 /* bottom halves */
 typedef void QEMUBHFunc(void *opaque);
 
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 1ff1ac6..6566e46 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -23,6 +23,7 @@
  */
 
 #import <Cocoa/Cocoa.h>
+#include <crt_externs.h>
 
 #include "qemu-common.h"
 #include "console.h"
@@ -61,7 +62,6 @@ typedef struct {
     int bitsPerPixel;
 } QEMUScreen;
 
-int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
 NSWindow *normalWindow;
 id cocoaView;
 static DisplayChangeListener *dcl;
@@ -794,7 +794,7 @@ static int cocoa_keycode_to_qemu(int keycode)
     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
 
     int status;
-    status = qemu_main(argc, argv);
+    status = qemu_main(argc, argv, *_NSGetEnviron());
     exit(status);
 }
 
@@ -876,7 +876,7 @@ int main (int argc, const char * argv[]) {
                 !strcmp(opt, "-nographic") ||
                 !strcmp(opt, "-version") ||
                 !strcmp(opt, "-curses")) {
-                return qemu_main(gArgc, gArgv);
+                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
             }
         }
     }
-- 
1.7.5.3

Re: [Qemu-devel] [PATCH] cocoa: Provide central qemu_main() prototype

[Alexandre Raymond]

Hi Andreas,

Just thinking out loud here : wouldn't it make more sense to put the
main() of each gui framework directly in its corresponding file and
select the right one in the makefile using the configure options?

so you'd have
-no gui -> ui/no_gui.c:main() -> qemu_main()    === compile with no_gui.c + vl.c
-sdl -> ui/sdl.c:main() -> qemu_main() === compile with sdl.c + vl.c
-cocoa -> ui/cocoa.m:main() -> qemu_main() === compile with cocoa.m + vl.c

with ui/no_gui.c, ui/sdl.c and ui/cocoa.m each having their own main():
----8<----
...
int main(...) {
    return qemu_main(....);
}
...
----8<----

and definitively rename main() to qemu_main() in vl.c ?

Alexandre

On Sun, May 29, 2011 at 3:58 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> This fixes a missing prototype warning in vl.c and obsoletes
> the prototype in cocoa.m. Adjust callers in cocoa.m to supply
> third argument, which is currently only used on Linux/ppc.
>
> The prototype is designed so that it could be shared with SDL
> and other frontends, if desired.
>
> Cc: Alexandre Raymond <cerbere@gmail.com>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  qemu-common.h |    5 +++++
>  ui/cocoa.m    |    6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-common.h b/qemu-common.h
> index b851b20..218289c 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -131,6 +131,11 @@ static inline char *realpath(const char *path, char *resolved_path)
>
>  #endif /* !defined(NEED_CPU_H) */
>
> +/* main function, renamed */
> +#if defined(CONFIG_COCOA)
> +int qemu_main(int argc, char **argv, char **envp);
> +#endif
> +
>  /* bottom halves */
>  typedef void QEMUBHFunc(void *opaque);
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 1ff1ac6..6566e46 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -23,6 +23,7 @@
>  */
>
>  #import <Cocoa/Cocoa.h>
> +#include <crt_externs.h>
>
>  #include "qemu-common.h"
>  #include "console.h"
> @@ -61,7 +62,6 @@ typedef struct {
>     int bitsPerPixel;
>  } QEMUScreen;
>
> -int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
>  NSWindow *normalWindow;
>  id cocoaView;
>  static DisplayChangeListener *dcl;
> @@ -794,7 +794,7 @@ static int cocoa_keycode_to_qemu(int keycode)
>     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
>
>     int status;
> -    status = qemu_main(argc, argv);
> +    status = qemu_main(argc, argv, *_NSGetEnviron());
>     exit(status);
>  }
>
> @@ -876,7 +876,7 @@ int main (int argc, const char * argv[]) {
>                 !strcmp(opt, "-nographic") ||
>                 !strcmp(opt, "-version") ||
>                 !strcmp(opt, "-curses")) {
> -                return qemu_main(gArgc, gArgv);
> +                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
>             }
>         }
>     }
> --
> 1.7.5.3
>
>

Re: [Qemu-devel] [PATCH] cocoa: Provide central qemu_main() prototype

[Andreas Färber]

Am 30.05.2011 um 01:53 schrieb Alexandre Raymond:

> Just thinking out loud here : wouldn't it make more sense to put the
> main() of each gui framework directly in its corresponding file and
> select the right one in the makefile using the configure options?
>
> so you'd have
> -no gui -> ui/no_gui.c:main() -> qemu_main()    === compile with  
> no_gui.c + vl.c
> -sdl -> ui/sdl.c:main() -> qemu_main() === compile with sdl.c + vl.c
> -cocoa -> ui/cocoa.m:main() -> qemu_main() === compile with cocoa.m  
> + vl.c
>
> with ui/no_gui.c, ui/sdl.c and ui/cocoa.m each having their own  
> main():
> ----8<----
> ...
> int main(...) {
>    return qemu_main(....);
> }
> ...
> ----8<----
>
> and definitively rename main() to qemu_main() in vl.c ?

Anthony, waiting on your comment here as it's an overall UI  
architectural question.

To me that sounds the wrong direction to fix this... The only frontend  
that forces another main() function on us seems to be SDL under some  
circumstances. For Cocoa that was a QEMU-internal choice.

Instead of always renaming our main() function, maybe we can introduce  
some general hooks from our main() that the frontends can use to  
initialize them? One hook would need to be before processing of  
options (since launching a Cocoa app may add some Cocoa-specific  
parameters from the desktop or AppleScript/Automator) and another one  
once the options are processed and it's clear what display mode we're  
in.

So should we go ahead with my patch for the next pull or do some  
reorganization touching all frontends?

Andreas

> On Sun, May 29, 2011 at 3:58 PM, Andreas Färber <andreas.faerber@web.de 
> > wrote:
>> This fixes a missing prototype warning in vl.c and obsoletes
>> the prototype in cocoa.m. Adjust callers in cocoa.m to supply
>> third argument, which is currently only used on Linux/ppc.
>>
>> The prototype is designed so that it could be shared with SDL
>> and other frontends, if desired.
>>
>> Cc: Alexandre Raymond <cerbere@gmail.com>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>>  qemu-common.h |    5 +++++
>>  ui/cocoa.m    |    6 +++---
>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/qemu-common.h b/qemu-common.h
>> index b851b20..218289c 100644
>> --- a/qemu-common.h
>> +++ b/qemu-common.h
>> @@ -131,6 +131,11 @@ static inline char *realpath(const char *path,  
>> char *resolved_path)
>>
>>  #endif /* !defined(NEED_CPU_H) */
>>
>> +/* main function, renamed */
>> +#if defined(CONFIG_COCOA)
>> +int qemu_main(int argc, char **argv, char **envp);
>> +#endif
>> +
>>  /* bottom halves */
>>  typedef void QEMUBHFunc(void *opaque);
>>
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 1ff1ac6..6566e46 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -23,6 +23,7 @@
>>  */
>>
>>  #import <Cocoa/Cocoa.h>
>> +#include <crt_externs.h>
>>
>>  #include "qemu-common.h"
>>  #include "console.h"
>> @@ -61,7 +62,6 @@ typedef struct {
>>     int bitsPerPixel;
>>  } QEMUScreen;
>>
>> -int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
>>  NSWindow *normalWindow;
>>  id cocoaView;
>>  static DisplayChangeListener *dcl;
>> @@ -794,7 +794,7 @@ static int cocoa_keycode_to_qemu(int keycode)
>>     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
>>
>>     int status;
>> -    status = qemu_main(argc, argv);
>> +    status = qemu_main(argc, argv, *_NSGetEnviron());
>>     exit(status);
>>  }
>>
>> @@ -876,7 +876,7 @@ int main (int argc, const char * argv[]) {
>>                 !strcmp(opt, "-nographic") ||
>>                 !strcmp(opt, "-version") ||
>>                 !strcmp(opt, "-curses")) {
>> -                return qemu_main(gArgc, gArgv);
>> +                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
>>             }
>>         }
>>     }
>> --
>> 1.7.5.3

Re: [Qemu-devel] [PATCH] cocoa: Provide central qemu_main() prototype

[Andreas Färber]

Am 29.05.2011 um 21:58 schrieb Andreas Färber:

> This fixes a missing prototype warning in vl.c and obsoletes
> the prototype in cocoa.m. Adjust callers in cocoa.m to supply
> third argument, which is currently only used on Linux/ppc.
>
> The prototype is designed so that it could be shared with SDL
> and other frontends, if desired.
>
> Cc: Alexandre Raymond <cerbere@gmail.com>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>

Applied to the cocoa branch.

Andreas

> ---
> qemu-common.h |    5 +++++
> ui/cocoa.m    |    6 +++---
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-common.h b/qemu-common.h
> index b851b20..218289c 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -131,6 +131,11 @@ static inline char *realpath(const char *path,  
> char *resolved_path)
>
> #endif /* !defined(NEED_CPU_H) */
>
> +/* main function, renamed */
> +#if defined(CONFIG_COCOA)
> +int qemu_main(int argc, char **argv, char **envp);
> +#endif
> +
> /* bottom halves */
> typedef void QEMUBHFunc(void *opaque);
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 1ff1ac6..6566e46 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -23,6 +23,7 @@
>  */
>
> #import <Cocoa/Cocoa.h>
> +#include <crt_externs.h>
>
> #include "qemu-common.h"
> #include "console.h"
> @@ -61,7 +62,6 @@ typedef struct {
>     int bitsPerPixel;
> } QEMUScreen;
>
> -int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
> NSWindow *normalWindow;
> id cocoaView;
> static DisplayChangeListener *dcl;
> @@ -794,7 +794,7 @@ static int cocoa_keycode_to_qemu(int keycode)
>     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
>
>     int status;
> -    status = qemu_main(argc, argv);
> +    status = qemu_main(argc, argv, *_NSGetEnviron());
>     exit(status);
> }
>
> @@ -876,7 +876,7 @@ int main (int argc, const char * argv[]) {
>                 !strcmp(opt, "-nographic") ||
>                 !strcmp(opt, "-version") ||
>                 !strcmp(opt, "-curses")) {
> -                return qemu_main(gArgc, gArgv);
> +                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
>             }
>         }
>     }
> -- 
> 1.7.5.3
>
>

[Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Alexandre Raymond]

----8<----
qemu/target-lm32/translate.c: In function ‘gen_intermediate_code_internal’:
qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects type ‘signed size_t’, but argument 4 has type ‘int’
----8<----

Both gen_opc_ptr and gen_opc_buf are "uint16_t *", so a simple '%d' should
be able to describe their relative difference.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 target-lm32/translate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index eb21158..0f69f27 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1132,7 +1132,7 @@ static void gen_intermediate_code_internal(CPUState *env,
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("\n");
         log_target_disas(pc_start, dc->pc - pc_start, 0);
-        qemu_log("\nisize=%d osize=%zd\n",
+        qemu_log("\nisize=%d osize=%d\n",
             dc->pc - pc_start, gen_opc_ptr - gen_opc_buf);
     }
 #endif
-- 
1.7.5

Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Stefan Weil]

Am 27.05.2011 19:22, schrieb Alexandre Raymond:
> ----8<----
> qemu/target-lm32/translate.c: In function 
> ‘gen_intermediate_code_internal’:
> qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects type 
> ‘signed size_t’, but argument 4 has type ‘int’
> ----8<----
>
> Both gen_opc_ptr and gen_opc_buf are "uint16_t *", so a simple '%d' should
> be able to describe their relative difference.
>
> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
> ---
> target-lm32/translate.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-lm32/translate.c b/target-lm32/translate.c
> index eb21158..0f69f27 100644
> --- a/target-lm32/translate.c
> +++ b/target-lm32/translate.c
> @@ -1132,7 +1132,7 @@ static void 
> gen_intermediate_code_internal(CPUState *env,
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
> qemu_log("\n");
> log_target_disas(pc_start, dc->pc - pc_start, 0);
> - qemu_log("\nisize=%d osize=%zd\n",
> + qemu_log("\nisize=%d osize=%d\n",
> dc->pc - pc_start, gen_opc_ptr - gen_opc_buf);
> }
> #endif

Nack.

The original code is correct, because the difference of two pointers
is always of type ssize_t (well, obviously not with your compiler,
but then I assume that your compiler is broken).

This pattern is quite common in QEMU, so it looks like there is
another problem here (otherwise you would get more errors of this kind).

Cheers,
Stefan W.

Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Markus Armbruster]

Stefan Weil <weil@mail.berlios.de> writes:

> Am 27.05.2011 19:22, schrieb Alexandre Raymond:
>> ----8<----
>> qemu/target-lm32/translate.c: In function
>> ‘gen_intermediate_code_internal’:
>> qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects
>> type ‘signed size_t’, but argument 4 has type ‘int’
>> ----8<----
>>
>> Both gen_opc_ptr and gen_opc_buf are "uint16_t *", so a simple '%d' should
>> be able to describe their relative difference.
>>
>> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
>> ---
>> target-lm32/translate.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-lm32/translate.c b/target-lm32/translate.c
>> index eb21158..0f69f27 100644
>> --- a/target-lm32/translate.c
>> +++ b/target-lm32/translate.c
>> @@ -1132,7 +1132,7 @@ static void
>> gen_intermediate_code_internal(CPUState *env,
>> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
>> qemu_log("\n");
>> log_target_disas(pc_start, dc->pc - pc_start, 0);
>> - qemu_log("\nisize=%d osize=%zd\n",
>> + qemu_log("\nisize=%d osize=%d\n",
>> dc->pc - pc_start, gen_opc_ptr - gen_opc_buf);
>> }
>> #endif
>
> Nack.
>
> The original code is correct, because the difference of two pointers
> is always of type ssize_t (well, obviously not with your compiler,
> but then I assume that your compiler is broken).

ISO/IEC 9899:1999 §6.5.6 on pointer subtraction:

    The size of the result is implementation-defined, and its type (a
    signed integer type) is ptrdiff_t defined in the <stddef.h> header.

The pedantically correct way to print a pointer difference is the 't'
type modifier.  Ibid. §7.19.6.1 on fprintf():

    t   Specifies  that a following d, i, o, u, x, or X conversion
        specifier applies to a ptrdiff_t or the corresponding unsigned
        integer type argument; or that a following n conversion
        specifier applies to a pointer to a ptrdiff_t argument.

ssize_t is POSIX, not ISO C.  It can differ from ptrdiff_t only if
ptrdiff_t has a different size than size_t, which would be kind of sick.
ISO C permits all kinds of sickness.  Whether tolerating a particular
sickness is worth our while is another question.

[...]

Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Stefan Weil]

Am 27.05.2011 21:11, schrieb Markus Armbruster:
> Stefan Weil <weil@mail.berlios.de> writes:
>
>> Am 27.05.2011 19:22, schrieb Alexandre Raymond:
>>> ----8<----
>>> qemu/target-lm32/translate.c: In function
>>> ‘gen_intermediate_code_internal’:
>>> qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects
>>> type ‘signed size_t’, but argument 4 has type ‘int’
>>> ----8<----
>>>
>>> Both gen_opc_ptr and gen_opc_buf are "uint16_t *", so a simple '%d' 
>>> should
>>> be able to describe their relative difference.
>>>
>>> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
>>> ---
>>> target-lm32/translate.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/target-lm32/translate.c b/target-lm32/translate.c
>>> index eb21158..0f69f27 100644
>>> --- a/target-lm32/translate.c
>>> +++ b/target-lm32/translate.c
>>> @@ -1132,7 +1132,7 @@ static void
>>> gen_intermediate_code_internal(CPUState *env,
>>> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
>>> qemu_log("\n");
>>> log_target_disas(pc_start, dc->pc - pc_start, 0);
>>> - qemu_log("\nisize=%d osize=%zd\n",
>>> + qemu_log("\nisize=%d osize=%d\n",
>>> dc->pc - pc_start, gen_opc_ptr - gen_opc_buf);
>>> }
>>> #endif
>>
>> Nack.
>>
>> The original code is correct, because the difference of two pointers
>> is always of type ssize_t (well, obviously not with your compiler,
>> but then I assume that your compiler is broken).
>
> ISO/IEC 9899:1999 §6.5.6 on pointer subtraction:
>
> The size of the result is implementation-defined, and its type (a
> signed integer type) is ptrdiff_t defined in the <stddef.h> header.
>
> The pedantically correct way to print a pointer difference is the 't'
> type modifier. Ibid. §7.19.6.1 on fprintf():
>
> t Specifies that a following d, i, o, u, x, or X conversion
> specifier applies to a ptrdiff_t or the corresponding unsigned
> integer type argument; or that a following n conversion
> specifier applies to a pointer to a ptrdiff_t argument.
>
> ssize_t is POSIX, not ISO C. It can differ from ptrdiff_t only if
> ptrdiff_t has a different size than size_t, which would be kind of sick.
> ISO C permits all kinds of sickness. Whether tolerating a particular
> sickness is worth our while is another question.
>
> [...]

That's correct. And ptrdiff_t needs %td.

Alexandre, could you please try %td instead of %zd?

Cheers,
Stefan W.

Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Alexandre Raymond]

Hi Stefan and Markus,

Thanks for your feedback :)

"%td" doesn't generate warnings on Linux nor on OSX.

Alexandre

On Fri, May 27, 2011 at 4:44 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 27.05.2011 21:11, schrieb Markus Armbruster:
>>
>> Stefan Weil <weil@mail.berlios.de> writes:
>>
>>> Am 27.05.2011 19:22, schrieb Alexandre Raymond:
>>>>
>>>> ----8<----
>>>> qemu/target-lm32/translate.c: In function
>>>> ‘gen_intermediate_code_internal’:
>>>> qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects
>>>> type ‘signed size_t’, but argument 4 has type ‘int’
>>>> ----8<----
>>>>
>>>> Both gen_opc_ptr and gen_opc_buf are "uint16_t *", so a simple '%d'
>>>> should
>>>> be able to describe their relative difference.
>>>>
>>>> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
>>>> ---
>>>> target-lm32/translate.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/target-lm32/translate.c b/target-lm32/translate.c
>>>> index eb21158..0f69f27 100644
>>>> --- a/target-lm32/translate.c
>>>> +++ b/target-lm32/translate.c
>>>> @@ -1132,7 +1132,7 @@ static void
>>>> gen_intermediate_code_internal(CPUState *env,
>>>> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
>>>> qemu_log("\n");
>>>> log_target_disas(pc_start, dc->pc - pc_start, 0);
>>>> - qemu_log("\nisize=%d osize=%zd\n",
>>>> + qemu_log("\nisize=%d osize=%d\n",
>>>> dc->pc - pc_start, gen_opc_ptr - gen_opc_buf);
>>>> }
>>>> #endif
>>>
>>> Nack.
>>>
>>> The original code is correct, because the difference of two pointers
>>> is always of type ssize_t (well, obviously not with your compiler,
>>> but then I assume that your compiler is broken).
>>
>> ISO/IEC 9899:1999 §6.5.6 on pointer subtraction:
>>
>> The size of the result is implementation-defined, and its type (a
>> signed integer type) is ptrdiff_t defined in the <stddef.h> header.
>>
>> The pedantically correct way to print a pointer difference is the 't'
>> type modifier. Ibid. §7.19.6.1 on fprintf():
>>
>> t Specifies that a following d, i, o, u, x, or X conversion
>> specifier applies to a ptrdiff_t or the corresponding unsigned
>> integer type argument; or that a following n conversion
>> specifier applies to a pointer to a ptrdiff_t argument.
>>
>> ssize_t is POSIX, not ISO C. It can differ from ptrdiff_t only if
>> ptrdiff_t has a different size than size_t, which would be kind of sick.
>> ISO C permits all kinds of sickness. Whether tolerating a particular
>> sickness is worth our while is another question.
>>
>> [...]
>
> That's correct. And ptrdiff_t needs %td.
>
> Alexandre, could you please try %td instead of %zd?
>
> Cheers,
> Stefan W.
>
>

Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Paolo Bonzini]

On 05/28/2011 12:10 AM, Alexandre Raymond wrote:
> Hi Stefan and Markus,
>
> Thanks for your feedback :)
>
> "%td" doesn't generate warnings on Linux nor on OSX.

Stefan, what about Windows?

Paolo

Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

[Stefan Weil]

Am 28.05.2011 08:55, schrieb Paolo Bonzini:
> On 05/28/2011 12:10 AM, Alexandre Raymond wrote:
>> Hi Stefan and Markus,
>>
>> Thanks for your feedback :)
>>
>> "%td" doesn't generate warnings on Linux nor on OSX.
>
> Stefan, what about Windows?
>
> Paolo

%td is ok for Windows, too.
Alexandre, please send an update of your patch using %td.

Stefan W.

[Qemu-devel] [PATCH 6/6] Avoid compilation warning regarding kvm under darwin

[Alexandre Raymond]

----8<----
qemu/target-s390x/helper.c:32:23: warning: linux/kvm.h: No such file or director
----8<----

kvm.h, which is included right after this line, already includes linux/kvm.h
with the proper CONFIG_KVM guard. Remove redundant include.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 target-s390x/helper.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index c79af46..5dc42f1 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -29,7 +29,6 @@
 #include "qemu-timer.h"
 
 #if !defined(CONFIG_USER_ONLY)
-#include <linux/kvm.h>
 #include "kvm.h"
 #endif
 
-- 
1.7.5

Re: [Qemu-devel] [PATCH 6/6] Avoid compilation warning regarding kvm under darwin

[Stefan Weil]

Am 27.05.2011 19:22, schrieb Alexandre Raymond:
> ----8<----
> qemu/target-s390x/helper.c:32:23: warning: linux/kvm.h: No such file 
> or director
> ----8<----
>
> kvm.h, which is included right after this line, already includes 
> linux/kvm.h
> with the proper CONFIG_KVM guard. Remove redundant include.
>
> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
> ---
> target-s390x/helper.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index c79af46..5dc42f1 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -29,7 +29,6 @@
> #include "qemu-timer.h"
>
> #if !defined(CONFIG_USER_ONLY)
> -#include <linux/kvm.h>
> #include "kvm.h"
> #endif

See http://patchwork.ozlabs.org/patch/97191/.

Regards,
Stefan W.

Re: [Qemu-devel] [PATCH 0/6] Fix compilation issues under darwin

[Andreas Färber]

Hello,

Am 27.05.2011 um 19:22 schrieb Alexandre Raymond:

> The following series contains trivial patches to fix several minor  
> issues
> encountered while compiling qemu under OSX 10.6.7.
>
> I used [./configure --disable-bsd-user --disable-darwin-user -- 
> enable-io-thread]
> to configure the build.

In the future, please cc me for Darwin. I'll review them tomorrow.

Thanks,
Andreas

Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/6] Fix compilation issues under darwin

[Stefan Hajnoczi]

On Sat, May 28, 2011 at 3:55 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Hello,
>
> Am 27.05.2011 um 19:22 schrieb Alexandre Raymond:
>
>> The following series contains trivial patches to fix several minor issues
>> encountered while compiling qemu under OSX 10.6.7.
>>
>> I used [./configure --disable-bsd-user --disable-darwin-user
>> --enable-io-thread]
>> to configure the build.
>
> In the future, please cc me for Darwin. I'll review them tomorrow.

Okay these patches will go through the Andreas' tree.  I won't be
merging any of them into qemu-trivial.

Stefan