[Qemu-devel] "Enable _FORTIFY_SOURCE=2" result in building failure for qemu-img.c

[Qemu-devel] "Enable _FORTIFY_SOURCE=2" result in building failure for qemu-img.c

[Sheng Yang]

Got this building failure:

In file included from qemu-img.c:68:                                                                                                                                                    
qemu-img-cmds.h:22:1: error: unterminated argument list invoking macro 
"printf"                                                                                                         
cc1: warnings being treated as errors
qemu-img.c: In function ‘help’:
qemu-img.c:95: warning: statement with no effect
qemu-img.c:71: error: expected ‘;’ before string constant
qemu-img.c:95: error: expected statement before ‘)’ token
make: *** [qemu-img.o] Error 1

And git bisect figure out the commit: 84958305, "Enable _FORTIFY_SOURCE=2".

And the code is here:

    printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice 
Bellard\n"
           "usage: qemu-img command [command options]\n"
           "QEMU disk image utility\n"
           "\n"
           "Command syntax:\n"
#define DEF(option, callback, arg_string)        \
           "  " arg_string "\n"
#include "qemu-img-cmds.h"
#undef DEF
#undef GEN_DOCS
....

Seems gcc take "printf" as a marco. I added a "#undef printf" before the line, 
then it works...

So any clue on what's happened and how to fix?

GCC version is 4.1.2.

-- 
regards
Yang, Sheng

[Qemu-devel] Re: "Enable _FORTIFY_SOURCE=2" result in building failure for qemu-img.c

[Paolo Bonzini]

On 02/04/2010 08:04 AM, Sheng Yang wrote:
> Got this building failure:
>
> In file included from qemu-img.c:68:
> qemu-img-cmds.h:22:1: error: unterminated argument list invoking macro
> "printf"
> cc1: warnings being treated as errors
> qemu-img.c: In function ‘help’:
> qemu-img.c:95: warning: statement with no effect
> qemu-img.c:71: error: expected ‘;’ before string constant
> qemu-img.c:95: error: expected statement before ‘)’ token
> make: *** [qemu-img.o] Error 1
>
> And git bisect figure out the commit: 84958305, "Enable _FORTIFY_SOURCE=2".
>
> And the code is here:
>
>      printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice
> Bellard\n"
>             "usage: qemu-img command [command options]\n"
>             "QEMU disk image utility\n"
>             "\n"
>             "Command syntax:\n"
> #define DEF(option, callback, arg_string)        \
>             "  " arg_string "\n"
> #include "qemu-img-cmds.h"
> #undef DEF
> #undef GEN_DOCS
> ....
>
> Seems gcc take "printf" as a marco. I added a "#undef printf" before the line,
> then it works...

It's glibc, not gcc.

I suggest moving the help message into a variable and changing the 
printf to "fputs (help_msg, stdout);".  Would you make a patch?

Paolo

Re: [Qemu-devel] "Enable _FORTIFY_SOURCE=2" result in building failure for qemu-img.c

[Jamie Lokier]

Sheng Yang wrote:
>     printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice 
> Bellard\n"
>            "usage: qemu-img command [command options]\n"
>            "QEMU disk image utility\n"
>            "\n"
>            "Command syntax:\n"
> #define DEF(option, callback, arg_string)        \
>            "  " arg_string "\n"
> #include "qemu-img-cmds.h"
> #undef DEF
> #undef GEN_DOCS
> ....
> 
> Seems gcc take "printf" as a marco. I added a "#undef printf" before the line, 
> then it works...
> 
> So any clue on what's happened and how to fix?

You can't have preprocessor directives inside the arguments of a macro
call.  Yes it's occasionally annoying like this.

You can prevent the macro call without #undef by writing:

    (printf)("qemu-img version " ...etc)

I'm not sure if Glibc is compliant with ISO C by
making printf into a macro that takes arguments.

Certain functions such as putchar() are specified as being allowed to
be macros, which implies the other standard functions aren't.

-- Jamie

[Qemu-devel] Re: "Enable _FORTIFY_SOURCE=2" result in building failure for qemu-img.c

[Juan Quintela]

Sheng Yang <sheng@linux.intel.com> wrote:
> Got this building failure:
>
> In file included from qemu-img.c:68:                                                                                                                                                    
> qemu-img-cmds.h:22:1: error: unterminated argument list invoking macro 
> "printf"                                                                                                         
> cc1: warnings being treated as errors
> qemu-img.c: In function ‘help’:
> qemu-img.c:95: warning: statement with no effect
> qemu-img.c:71: error: expected ‘;’ before string constant
> qemu-img.c:95: error: expected statement before ‘)’ token
> make: *** [qemu-img.o] Error 1
>
> And git bisect figure out the commit: 84958305, "Enable _FORTIFY_SOURCE=2".
>
> And the code is here:
>
>     printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice 
> Bellard\n"
>            "usage: qemu-img command [command options]\n"
>            "QEMU disk image utility\n"
>            "\n"
>            "Command syntax:\n"
> #define DEF(option, callback, arg_string)        \
>            "  " arg_string "\n"
> #include "qemu-img-cmds.h"
> #undef DEF
> #undef GEN_DOCS
> ....
>
> Seems gcc take "printf" as a marco. I added a "#undef printf" before the line, 
> then it works...
>
> So any clue on what's happened and how to fix?
>
> GCC version is 4.1.2.

I looks like that GCC + _FORTIFY_SOURCE=2 is broken on your platfrom.

What is better, to remove the multi-line string (it appears that is the
only one there) or to make a test to check that _FORTIFY_SOURCE works on
platforms.

Anthony, what do you preffer?

Later, Juan.

[Qemu-devel] [PATCH] qemu-img: avoid preprocessor directives in a printf call

[Paolo Bonzini]

Other choices include using "(printf)", but this one is not bad in
terms of readability.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-img.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index cbba4fc..eb5c0f0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -58,7 +58,8 @@ static void format_print(void *opaque, const char *name)
 /* Please keep in synch with qemu-img.texi */
 static void help(void)
 {
-    printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice Bellard\n"
+    const char *help_msg =
+           "qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice Bellard\n"
            "usage: qemu-img command [command options]\n"
            "QEMU disk image utility\n"
            "\n"
@@ -91,9 +92,9 @@ static void help(void)
            "  '-a' applies a snapshot (revert disk to saved state)\n"
            "  '-c' creates a snapshot\n"
            "  '-d' deletes a snapshot\n"
-           "  '-l' lists all snapshots in the given image\n"
-           );
-    printf("\nSupported formats:");
+           "  '-l' lists all snapshots in the given image\n";
+
+    printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
     printf("\n");
     exit(1);
-- 
1.6.6

[Qemu-devel] Re: [PATCH] qemu-img: avoid preprocessor directives in a printf call

[Sheng Yang]

On Thursday 04 February 2010 21:29:40 Paolo Bonzini wrote:
> Other choices include using "(printf)", but this one is not bad in
> terms of readability.

This works well. :)

Acked-by: Sheng Yang <sheng@linux.intel.com>
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

And there is two more...

One is:

readline.c:31:1: error: "printf" redefined
In file included from /usr/include/stdio.h:844,
                 from qemu-common.h:21,
                 from readline.h:4,
                 from readline.c:24:
/usr/include/bits/stdio2.h:65:1: error: this is the location of the previous 
definition
make: *** [readline.o] Error 1

Of course, what you can see in readline.c is:

#define printf do_not_use_printf

I guess it's OK to remove this?

And there is last one in help() of vl.c...

After resolve these three, the build is fine now.

Paolo, could you address the other two issues as well in your patch? Thanks!

-- 
regards
Yang, Sheng

[Qemu-devel] [PATCH 0/4] Fix printf calls embedding preprocessor directives

[Paolo Bonzini]

Patch 1 is the same I sent earlier.  Patches 2 and 3/4 fix the
other two problems by Sheng (tip: next time use "make -k" and
report all problems in a single message).

Paolo Bonzini (4):
  qemu-img: avoid preprocessor directives in a printf call
  cope with printf macro definition in readline.c
  do not interpolate % from vl.c to qemu-options.h
  vl.c: avoid preprocessor directives in a printf call

 qemu-img.c      |    9 +++++----
 qemu-options.hx |   15 ++++++++-------
 readline.c      |    1 +
 vl.c            |   23 +++++++++--------------
 4 files changed, 23 insertions(+), 25 deletions(-)

[Qemu-devel] [PATCH 1/4] qemu-img: avoid preprocessor directives in a printf call

[Paolo Bonzini]

Other choices include using "(printf)", but this one is not bad in
terms of readability.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-img.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index cbba4fc..eb5c0f0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -58,7 +58,8 @@ static void format_print(void *opaque, const char *name)
 /* Please keep in synch with qemu-img.texi */
 static void help(void)
 {
-    printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice Bellard\n"
+    const char *help_msg =
+           "qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice Bellard\n"
            "usage: qemu-img command [command options]\n"
            "QEMU disk image utility\n"
            "\n"
@@ -91,9 +92,9 @@ static void help(void)
            "  '-a' applies a snapshot (revert disk to saved state)\n"
            "  '-c' creates a snapshot\n"
            "  '-d' deletes a snapshot\n"
-           "  '-l' lists all snapshots in the given image\n"
-           );
-    printf("\nSupported formats:");
+           "  '-l' lists all snapshots in the given image\n";
+
+    printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
     printf("\n");
     exit(1);
-- 
1.6.6

[Qemu-devel] [PATCH 2/4] cope with printf macro definition in readline.c

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 readline.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/readline.c b/readline.c
index 7834af0..92f9cd1 100644
--- a/readline.c
+++ b/readline.c
@@ -28,6 +28,7 @@
 #define IS_ESC  1
 #define IS_CSI  2
 
+#undef printf
 #define printf do_not_use_printf
 
 void readline_show_prompt(ReadLineState *rs)
-- 
1.6.6

[Qemu-devel] [PATCH 3/4] do not interpolate % from vl.c to qemu-options.h

[Paolo Bonzini]

Since qemu-options.h is only used in vl.c, we can avoid using
brittle interpolation from a generated file.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx |   11 ++++++-----
 vl.c            |    9 +--------
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 4c1bcfb..2fa9ed4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -275,7 +275,8 @@ the write back by pressing @key{C-a s} (@pxref{disk_images}).
 ETEXI
 
 DEF("m", HAS_ARG, QEMU_OPTION_m,
-    "-m megs         set virtual RAM size to megs MB [default=%d]\n")
+    "-m megs         set virtual RAM size to megs MB [default="
+    stringify(DEFAULT_RAM_SIZE) "]\n")
 STEXI
 @item -m @var{megs}
 Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB.  Optionally,
@@ -830,8 +831,8 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
 #else
     "-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off]\n"
     "                connect the host TAP network interface to VLAN 'n' and use the\n"
-    "                network scripts 'file' (default=%s)\n"
-    "                and 'dfile' (default=%s)\n"
+    "                network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
+    "                and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
     "                use '[down]script=no' to disable script execution\n"
     "                use 'fd=h' to connect to an already opened TAP interface\n"
     "                use 'sndbuf=nbytes' to limit the size of the send buffer (the\n"
@@ -1664,7 +1665,7 @@ within gdb and establish the connection via a pipe:
 ETEXI
 
 DEF("s", 0, QEMU_OPTION_s, \
-    "-s              shorthand for -gdb tcp::%s\n")
+    "-s              shorthand for -gdb tcp::" DEFAULT_GDBSTUB_PORT "\n")
 STEXI
 @item -s
 Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
@@ -1672,7 +1673,7 @@ Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
 ETEXI
 
 DEF("d", HAS_ARG, QEMU_OPTION_d, \
-    "-d item1,...    output log to %s (use -d ? for a list of log items)\n")
+    "-d item1,...    output log to /tmp/qemu.log (use -d ? for a list of log items)\n")
 STEXI
 @item -d
 Output log in /tmp/qemu.log
diff --git a/vl.c b/vl.c
index 39833fc..3d2de7b 100644
--- a/vl.c
+++ b/vl.c
@@ -4041,14 +4041,7 @@ static void help(int exitcode)
            "\n"
            "When using -nographic, press 'ctrl-a h' to get some help.\n"
            ,
-           "qemu",
-           DEFAULT_RAM_SIZE,
-#ifndef _WIN32
-           DEFAULT_NETWORK_SCRIPT,
-           DEFAULT_NETWORK_DOWN_SCRIPT,
-#endif
-           DEFAULT_GDBSTUB_PORT,
-           "/tmp/qemu.log");
+           "qemu");
     exit(exitcode);
 }
 
-- 
1.6.6

[Qemu-devel] [PATCH 4/4] vl.c: avoid preprocessor directives in a printf call

[Paolo Bonzini]

Similar to the qemu-img.c patch, but I also have to unescape remaining
% signs in qemu-options.hx.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx |    4 ++--
 vl.c            |   18 ++++++++++--------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 2fa9ed4..bb2d4fa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -430,7 +430,7 @@ Also optionally set the top visible process name in Linux.
 ETEXI
 
 DEF("uuid", HAS_ARG, QEMU_OPTION_uuid,
-    "-uuid %%08x-%%04x-%%04x-%%04x-%%012x\n"
+    "-uuid %08x-%04x-%04x-%04x-%012x\n"
     "                specify machine UUID\n")
 STEXI
 @item -uuid @var{uuid}
@@ -773,7 +773,7 @@ ETEXI
 DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
     "-smbios file=binary\n"
     "                load SMBIOS entry from binary file\n"
-    "-smbios type=0[,vendor=str][,version=str][,date=str][,release=%%d.%%d]\n"
+    "-smbios type=0[,vendor=str][,version=str][,date=str][,release=%d.%d]\n"
     "                specify SMBIOS type 0 fields\n"
     "-smbios type=1[,manufacturer=str][,product=str][,version=str][,serial=str]\n"
     "              [,uuid=uuid][,sku=str][,family=str]\n"
diff --git a/vl.c b/vl.c
index 3d2de7b..057046e 100644
--- a/vl.c
+++ b/vl.c
@@ -4021,11 +4021,7 @@ static void version(void)
 
 static void help(int exitcode)
 {
-    version();
-    printf("usage: %s [options] [disk_image]\n"
-           "\n"
-           "'disk_image' is a raw hard image image for IDE hard disk 0\n"
-           "\n"
+    const char *options_help =
 #define DEF(option, opt_arg, opt_enum, opt_help)        \
            opt_help
 #define DEFHEADING(text) stringify(text) "\n"
@@ -4033,15 +4029,21 @@ static void help(int exitcode)
 #undef DEF
 #undef DEFHEADING
 #undef GEN_DOCS
+        ;
+    version();
+    printf("usage: %s [options] [disk_image]\n"
+           "\n"
+           "'disk_image' is a raw hard image image for IDE hard disk 0\n"
            "\n"
+           "%s\n"
            "During emulation, the following keys are useful:\n"
            "ctrl-alt-f      toggle full screen\n"
            "ctrl-alt-n      switch to virtual console 'n'\n"
            "ctrl-alt        toggle mouse and keyboard grab\n"
            "\n"
-           "When using -nographic, press 'ctrl-a h' to get some help.\n"
-           ,
-           "qemu");
+           "When using -nographic, press 'ctrl-a h' to get some help.\n",
+           "qemu",
+           options_help);
     exit(exitcode);
 }
 
-- 
1.6.6

Re: [Qemu-devel] "Enable _FORTIFY_SOURCE=2" result in building failure for qemu-img.c

[Markus Armbruster]

Jamie Lokier <jamie@shareable.org> writes:

> Sheng Yang wrote:
>>     printf("qemu-img version " QEMU_VERSION ", Copyright (c) 2004-2008 Fabrice 
>> Bellard\n"
>>            "usage: qemu-img command [command options]\n"
>>            "QEMU disk image utility\n"
>>            "\n"
>>            "Command syntax:\n"
>> #define DEF(option, callback, arg_string)        \
>>            "  " arg_string "\n"
>> #include "qemu-img-cmds.h"
>> #undef DEF
>> #undef GEN_DOCS
>> ....
>> 
>> Seems gcc take "printf" as a marco. I added a "#undef printf" before the line, 
>> then it works...
>> 
>> So any clue on what's happened and how to fix?
>
> You can't have preprocessor directives inside the arguments of a macro
> call.  Yes it's occasionally annoying like this.
>
> You can prevent the macro call without #undef by writing:
>
>     (printf)("qemu-img version " ...etc)

Yup.

> I'm not sure if Glibc is compliant with ISO C by
> making printf into a macro that takes arguments.

It is.  7.1.4 "Use of library functions" says "Any function declared in
a header may be additionally implemented as a function-like macro
defined in the header".

> Certain functions such as putchar() are specified as being allowed to
> be macros, which implies the other standard functions aren't.

You mean putc(), don't you?  7.19.7.8:

    The putc function is equivalent to fputc, except that if it is
    implemented as a macro, it may evaluate stream more than once, so
    that argument should never be an expression with side effects.

The general license-to-macro in 7.1.4 wouldn't allow a macro that
evaluates the argument more than once, so we need a special license
here.

[Qemu-devel] Re: [PATCH 0/4] Fix printf calls embedding preprocessor directives

[Sheng Yang]

On Thursday 04 February 2010 23:49:55 Paolo Bonzini wrote:
> Patch 1 is the same I sent earlier.  Patches 2 and 3/4 fix the
> other two problems by Sheng (tip: next time use "make -k" and
> report all problems in a single message).
> 
> Paolo Bonzini (4):
>   qemu-img: avoid preprocessor directives in a printf call
>   cope with printf macro definition in readline.c
>   do not interpolate % from vl.c to qemu-options.h
>   vl.c: avoid preprocessor directives in a printf call
> 
>  qemu-img.c      |    9 +++++----
>  qemu-options.hx |   15 ++++++++-------
>  readline.c      |    1 +
>  vl.c            |   23 +++++++++--------------
>  4 files changed, 23 insertions(+), 25 deletions(-)
> 
Works fine with me. Thanks. :)

But:

diff --git a/vl.c b/vl.c
index 39833fc..3d2de7b 100644
--- a/vl.c
+++ b/vl.c
@@ -4041,14 +4041,7 @@ static void help(int exitcode)
            "\n"
            "When using -nographic, press 'ctrl-a h' to get some help.\n"
            ,
-           "qemu",
-           DEFAULT_RAM_SIZE,
-#ifndef _WIN32
-           DEFAULT_NETWORK_SCRIPT,
-           DEFAULT_NETWORK_DOWN_SCRIPT,
-#endif
-           DEFAULT_GDBSTUB_PORT,
-           "/tmp/qemu.log");
+           "qemu");
     exit(exitcode);
 }

Is it proper to remove #ifndef _WIN32 there?

-- 
regards
Yang, Sheng

Re: [Qemu-devel] Re: [PATCH 0/4] Fix printf calls embedding preprocessor directives

[Kevin Wolf]

Am 05.02.2010 03:26, schrieb Sheng Yang:
> On Thursday 04 February 2010 23:49:55 Paolo Bonzini wrote:
>> Patch 1 is the same I sent earlier.  Patches 2 and 3/4 fix the
>> other two problems by Sheng (tip: next time use "make -k" and
>> report all problems in a single message).
>>
>> Paolo Bonzini (4):
>>   qemu-img: avoid preprocessor directives in a printf call
>>   cope with printf macro definition in readline.c
>>   do not interpolate % from vl.c to qemu-options.h
>>   vl.c: avoid preprocessor directives in a printf call
>>
>>  qemu-img.c      |    9 +++++----
>>  qemu-options.hx |   15 ++++++++-------
>>  readline.c      |    1 +
>>  vl.c            |   23 +++++++++--------------
>>  4 files changed, 23 insertions(+), 25 deletions(-)
>>
> Works fine with me. Thanks. :)
> 
> But:
> 
> diff --git a/vl.c b/vl.c
> index 39833fc..3d2de7b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4041,14 +4041,7 @@ static void help(int exitcode)
>             "\n"
>             "When using -nographic, press 'ctrl-a h' to get some help.\n"
>             ,
> -           "qemu",
> -           DEFAULT_RAM_SIZE,
> -#ifndef _WIN32
> -           DEFAULT_NETWORK_SCRIPT,
> -           DEFAULT_NETWORK_DOWN_SCRIPT,
> -#endif
> -           DEFAULT_GDBSTUB_PORT,
> -           "/tmp/qemu.log");
> +           "qemu");
>      exit(exitcode);
>  }
> 
> Is it proper to remove #ifndef _WIN32 there?

These lines are only moved into another #ifdef (in qemu-options.hx), so
this looks right to me.

Kevin

[Qemu-devel] Re: [PATCH 0/4] Fix printf calls embedding preprocessor directives

[Paolo Bonzini]

On 02/05/2010 03:26 AM, Sheng Yang wrote:
> -#ifndef _WIN32
> -           DEFAULT_NETWORK_SCRIPT,
> -           DEFAULT_NETWORK_DOWN_SCRIPT,
> -#endif
> -           DEFAULT_GDBSTUB_PORT,
> -           "/tmp/qemu.log");
> +           "qemu");
>       exit(exitcode);
>   }
>
> Is it proper to remove #ifndef _WIN32 there?

Yes, this matched a #ifndef in qemu-options.hx that remains there.

Paolo