diff mbox

[2/2] target-ppc: Cast ssize_t to size_t before printing with %zx

Message ID 1419373336-17800-3-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell Dec. 23, 2014, 10:22 p.m. UTC
The mingw32 compiler complains about trying to print variables of type
ssize_t with the %z format string specifier. Since we're printing it
as unsigned hex anyway, cast to size_t to silence the warning.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I suspect that this is a compiler bug, but this is the only instance
in the codebase so I'm prepared to work around it to get to a point
where we can turn on warnings-as-errors for w32, because some of the
w32-specific errors really are flagging up issues we need to fix.
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Weil Dec. 23, 2014, 10:36 p.m. UTC | #1
Am 23.12.2014 um 23:22 schrieb Peter Maydell:
> The mingw32 compiler complains about trying to print variables of type
> ssize_t with the %z format string specifier. Since we're printing it
> as unsigned hex anyway, cast to size_t to silence the warning.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I suspect that this is a compiler bug, but this is the only instance
> in the codebase so I'm prepared to work around it to get to a point
> where we can turn on warnings-as-errors for w32, because some of the
> w32-specific errors really are flagging up issues we need to fix.
> ---
>   hw/ppc/spapr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 08401e0..9aaa800 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>       }
>       if (spapr->rtas_size > RTAS_MAX_SIZE) {
>           hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
> -                 spapr->rtas_size, RTAS_MAX_SIZE);
> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>           exit(1);
>       }
>       g_free(filename);

Which compiler did you use? I get no warning with Debian's 
x86_64-w64-mingw32-gcc 4.6.3 or
native MinGW-w32 compilers.

Stefan
Peter Maydell Dec. 23, 2014, 10:47 p.m. UTC | #2
On 23 December 2014 at 22:36, Stefan Weil <sw@weilnetz.de> wrote:
> Am 23.12.2014 um 23:22 schrieb Peter Maydell:
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>>       }
>>       if (spapr->rtas_size > RTAS_MAX_SIZE) {
>>           hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
>> -                 spapr->rtas_size, RTAS_MAX_SIZE);
>> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>>           exit(1);
>>       }
>>       g_free(filename);
>
>
> Which compiler did you use? I get no warning with Debian's
> x86_64-w64-mingw32-gcc 4.6.3 or
> native MinGW-w32 compilers.

$ i586-mingw32msvc-gcc --version
i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2)

Yes, this is ancient... it's from the Debian mingw32 package.
I just use this for compile testing, not for trying to run.
I should probably switch to the w64 compiler for build tests;
I forget now if there was a reason why I hadn't.

I suspect, as I say, that this is just a generic old-gcc bug,
but it's the only one in the codebase, so it seems easiest
just to fix it.

-- PMM
Eric Blake Dec. 23, 2014, 10:50 p.m. UTC | #3
On 12/23/2014 03:22 PM, Peter Maydell wrote:
> The mingw32 compiler complains about trying to print variables of type
> ssize_t with the %z format string specifier. Since we're printing it
> as unsigned hex anyway, cast to size_t to silence the warning.

Hmm.  I wonder if mingw headers are mixing 'long' and 'unsigned int' (or
maybe 'unsigned long' and 'int') for the underlying definitions of
'ssize_t' vs. 'size_t'; both are 32-bits on the platform, but a
difference in underlying type would definitely explain why you get a
compiler warning if you use %zd with size_t or %zu with ssize_t.  On the
other hand, the warning I've most often seen is that native windows
completely lacks %z support, so gcc is smart enough to warn that ANY use
of %z is not going to work, for a function marked
__attribute__((__printf__)) on mingw.

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I suspect that this is a compiler bug, but this is the only instance
> in the codebase so I'm prepared to work around it to get to a point
> where we can turn on warnings-as-errors for w32, because some of the
> w32-specific errors really are flagging up issues we need to fix.

With new enough mingw headers, the alternative solution is to guarantee
that #__USE_MINGW_ANSI_STDIO is defined before including standard
headers, then %z, %j, %lld, and other required constructs magically work
(because you are no longer using the native printf, but the fixed mingw
shim).  Once you do that, you want to use
__attribute__((__gnu_printf__)) instead of __attribute__((__printf__))
in order to teach gcc that yes, your use of %z is really going to work.

For some more background reading:
http://sourceforge.net/p/mingw-w64/wiki2/gnu%20printf/
https://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00029.html
> ---
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 08401e0..9aaa800 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>      }
>      if (spapr->rtas_size > RTAS_MAX_SIZE) {
>          hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
> -                 spapr->rtas_size, RTAS_MAX_SIZE);
> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);

What's the actual compiler warning you got? This feels like it won't
work if the real problem is the compiler telling you that %z is unknown;
conversely, if it is a mismatch between 'size_t' vs. 'ssize_t', we
probably ought to report it to the mingw folks to fix their environment
to use the same underlying type for signed vs. unsigned sizes.
Peter Maydell Dec. 23, 2014, 11:09 p.m. UTC | #4
On 23 December 2014 at 22:50, Eric Blake <eblake@redhat.com> wrote:
> What's the actual compiler warning you got? This feels like it won't
> work if the real problem is the compiler telling you that %z is unknown;
> conversely, if it is a mismatch between 'size_t' vs. 'ssize_t', we
> probably ought to report it to the mingw folks to fix their environment
> to use the same underlying type for signed vs. unsigned sizes.

It complains:
hw/ppc/spapr.c: In function ‘ppc_spapr_init’:
hw/ppc/spapr.c:1441: warning: format ‘%zx’ expects type ‘size_t’, but
argument 2 has type ‘ssize_t’

and unsurprisingly casting to size_t suppresses it.

%zd gets me:

hw/ppc/spapr.c:1441: warning: format ‘%zd’ expects type ‘signed
size_t’, but argument 2 has type ‘ssize_t’

which is curious. (But we want the hex output anyway...)

Grepping through the headers suggests we have

typedef long _ssize_t;
typedef _ssize_t ssize_t;

I think 'size_t' ends up 'long unsigned int', though stddef.h is
tricky to interpret because of all the macros.

In any case, AIUI mingw isn't really maintained any more, and
so the answer is either "do what is needed to shut up the compiler"
or "declare mingw actually not supported and require mingw-w64"...

-- PMM
Stefan Weil Dec. 24, 2014, 9:47 a.m. UTC | #5
Am 23.12.2014 um 23:47 schrieb Peter Maydell:
> On 23 December 2014 at 22:36, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 23.12.2014 um 23:22 schrieb Peter Maydell:
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1438,7 +1438,7 @@ static void ppc_spapr_init(MachineState *machine)
>>>       }
>>>       if (spapr->rtas_size > RTAS_MAX_SIZE) {
>>>           hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
>>> -                 spapr->rtas_size, RTAS_MAX_SIZE);
>>> +                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>>>           exit(1);
>>>       }
>>>       g_free(filename);
>>
>>
>> Which compiler did you use? I get no warning with Debian's
>> x86_64-w64-mingw32-gcc 4.6.3 or
>> native MinGW-w32 compilers.
> 
> $ i586-mingw32msvc-gcc --version
> i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2)
> 
> Yes, this is ancient... it's from the Debian mingw32 package.
> I just use this for compile testing, not for trying to run.
> I should probably switch to the w64 compiler for build tests;
> I forget now if there was a reason why I hadn't.
> 
> I suspect, as I say, that this is just a generic old-gcc bug,
> but it's the only one in the codebase, so it seems easiest
> just to fix it.
> 
> -- PMM

"git grep" finds 6 "%zx", 66 "%zu" and 80 "%zd" in my QEMU source tree.
Some of those are in debug traces, others are conditionally compiled
depending on your configuration. Are you sure that none of those needs
the same kind of modification, too? I don't thing fixing one of them for
an ancient compiler is worth the trouble of explaining why there is a
type cast.

What about using hwaddr instead of ssize_t for rtas_size (and
HWADDR_PRIx in the format string)? It looks like that would be more
consistent with the rest of the code. See for example the arguments of
function spapr_finalize_fdt.

Stefan
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 08401e0..9aaa800 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1438,7 +1438,7 @@  static void ppc_spapr_init(MachineState *machine)
     }
     if (spapr->rtas_size > RTAS_MAX_SIZE) {
         hw_error("RTAS too big ! 0x%zx bytes (max is 0x%x)\n",
-                 spapr->rtas_size, RTAS_MAX_SIZE);
+                 (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
         exit(1);
     }
     g_free(filename);