Patchwork PPC: dbdma: macio: Fix format specifiers (build regression)

login
register
mail settings
Submitter Stefan Weil
Date July 12, 2013, 4:48 p.m.
Message ID <1373647719-32757-1-git-send-email-sw@weilnetz.de>
Download mbox | patch
Permalink /patch/258774/
State Accepted
Headers show

Comments

Stefan Weil - July 12, 2013, 4:48 p.m.
Fix a number of warnings for 32 bit builds (tested on MingW and Linux):

  CC    hw/ide/macio.o
qemu/hw/ide/macio.c: In function 'pmac_ide_atapi_transfer_cb':
qemu/hw/ide/macio.c:134:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
qemu/hw/ide/macio.c: In function 'pmac_ide_transfer_cb':
qemu/hw/ide/macio.c:215:5: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'int64_t' [-Werror=format]
qemu/hw/ide/macio.c:222:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
qemu/hw/ide/macio.c:264:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
cc1: all warnings being treated as errors
make: *** [hw/ide/macio.o] Error 1

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---



Hi Anthony,

the patch fixes a build regression which was introduced today.
Could you please apply it without waiting for the next pull requests?

Thanks,
Stefan



 hw/ide/macio.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Stefan Weil - July 16, 2013, 5:54 a.m.
Am 12.07.2013 18:48, schrieb Stefan Weil:
> Fix a number of warnings for 32 bit builds (tested on MingW and Linux):
>
>   CC    hw/ide/macio.o
> qemu/hw/ide/macio.c: In function 'pmac_ide_atapi_transfer_cb':
> qemu/hw/ide/macio.c:134:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
> qemu/hw/ide/macio.c: In function 'pmac_ide_transfer_cb':
> qemu/hw/ide/macio.c:215:5: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'int64_t' [-Werror=format]
> qemu/hw/ide/macio.c:222:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
> qemu/hw/ide/macio.c:264:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
> cc1: all warnings being treated as errors
> make: *** [hw/ide/macio.o] Error 1
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
>
>
> Hi Anthony,
>
> the patch fixes a build regression which was introduced today.
> Could you please apply it without waiting for the next pull requests?
>
> Thanks,
> Stefan
>
>
>
>  hw/ide/macio.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index 38ad924..ef4ba2b 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -131,7 +131,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
>          int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
>          int nsector = io->len >> 9;
>  
> -        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
> +        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
>                        unaligned, io->addr + io->len - unaligned);
>  
>          bdrv_read(s->bs, sector_num + nsector, io->remainder, 1);
> @@ -212,14 +212,15 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>          s->nsector -= n;
>      }
>  
> -    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d sector_num: %ld\n",
> +    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d "
> +                  "sector_num: %" PRId64 "\n",
>                    io->remainder_len, io->len, s->nsector, sector_num);
>      if (io->remainder_len && io->len) {
>          /* guest wants the rest of its previous transfer */
>          int remainder_len = MIN(io->remainder_len, io->len);
>          uint8_t *p = &io->remainder[0x200 - remainder_len];
>  
> -        MACIO_DPRINTF("copying remainder %d bytes at %#lx\n",
> +        MACIO_DPRINTF("copying remainder %d bytes at %#" HWADDR_PRIx "\n",
>                        remainder_len, io->addr);
>  
>          switch (s->dma_cmd) {
> @@ -261,7 +262,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>      if (unaligned) {
>          int nsector = io->len >> 9;
>  
> -        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
> +        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
>                        unaligned, io->addr + io->len - unaligned);
>  
>          switch (s->dma_cmd) {


Ping?
Stefan Weil - July 18, 2013, 7:49 p.m.
Am 16.07.2013 07:54, schrieb Stefan Weil:
> Am 12.07.2013 18:48, schrieb Stefan Weil:
>> Fix a number of warnings for 32 bit builds (tested on MingW and Linux):
>>
>>   CC    hw/ide/macio.o
>> qemu/hw/ide/macio.c: In function 'pmac_ide_atapi_transfer_cb':
>> qemu/hw/ide/macio.c:134:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>> qemu/hw/ide/macio.c: In function 'pmac_ide_transfer_cb':
>> qemu/hw/ide/macio.c:215:5: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'int64_t' [-Werror=format]
>> qemu/hw/ide/macio.c:222:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>> qemu/hw/ide/macio.c:264:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>> cc1: all warnings being treated as errors
>> make: *** [hw/ide/macio.o] Error 1
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>>
>>
>> Hi Anthony,
>>
>> the patch fixes a build regression which was introduced today.
>> Could you please apply it without waiting for the next pull requests?
>>
>> Thanks,
>> Stefan
>>
>>
>>
>>  hw/ide/macio.c |    9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
>> index 38ad924..ef4ba2b 100644
>> --- a/hw/ide/macio.c
>> +++ b/hw/ide/macio.c
>> @@ -131,7 +131,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
>>          int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
>>          int nsector = io->len >> 9;
>>  
>> -        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
>> +        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
>>                        unaligned, io->addr + io->len - unaligned);
>>  
>>          bdrv_read(s->bs, sector_num + nsector, io->remainder, 1);
>> @@ -212,14 +212,15 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>>          s->nsector -= n;
>>      }
>>  
>> -    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d sector_num: %ld\n",
>> +    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d "
>> +                  "sector_num: %" PRId64 "\n",
>>                    io->remainder_len, io->len, s->nsector, sector_num);
>>      if (io->remainder_len && io->len) {
>>          /* guest wants the rest of its previous transfer */
>>          int remainder_len = MIN(io->remainder_len, io->len);
>>          uint8_t *p = &io->remainder[0x200 - remainder_len];
>>  
>> -        MACIO_DPRINTF("copying remainder %d bytes at %#lx\n",
>> +        MACIO_DPRINTF("copying remainder %d bytes at %#" HWADDR_PRIx "\n",
>>                        remainder_len, io->addr);
>>  
>>          switch (s->dma_cmd) {
>> @@ -261,7 +262,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>>      if (unaligned) {
>>          int nsector = io->len >> 9;
>>  
>> -        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
>> +        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
>>                        unaligned, io->addr + io->len - unaligned);
>>  
>>          switch (s->dma_cmd) {
>
> Ping?


Ping²?
Alexander Graf - July 18, 2013, 10:40 p.m.
On 18.07.2013, at 21:49, Stefan Weil wrote:

> Am 16.07.2013 07:54, schrieb Stefan Weil:
>> Am 12.07.2013 18:48, schrieb Stefan Weil:
>>> Fix a number of warnings for 32 bit builds (tested on MingW and Linux):
>>> 
>>>  CC    hw/ide/macio.o
>>> qemu/hw/ide/macio.c: In function 'pmac_ide_atapi_transfer_cb':
>>> qemu/hw/ide/macio.c:134:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>>> qemu/hw/ide/macio.c: In function 'pmac_ide_transfer_cb':
>>> qemu/hw/ide/macio.c:215:5: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'int64_t' [-Werror=format]
>>> qemu/hw/ide/macio.c:222:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>>> qemu/hw/ide/macio.c:264:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>>> cc1: all warnings being treated as errors
>>> make: *** [hw/ide/macio.o] Error 1
>>> 
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>> 
>>> 
>>> 
>>> Hi Anthony,
>>> 
>>> the patch fixes a build regression which was introduced today.
>>> Could you please apply it without waiting for the next pull requests?
>>> 
>>> Thanks,
>>> Stefan
>>> 
>>> 
>>> 
>>> hw/ide/macio.c |    9 +++++----
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
>>> index 38ad924..ef4ba2b 100644
>>> --- a/hw/ide/macio.c
>>> +++ b/hw/ide/macio.c
>>> @@ -131,7 +131,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
>>>         int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
>>>         int nsector = io->len >> 9;
>>> 
>>> -        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
>>> +        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
>>>                       unaligned, io->addr + io->len - unaligned);
>>> 
>>>         bdrv_read(s->bs, sector_num + nsector, io->remainder, 1);
>>> @@ -212,14 +212,15 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>>>         s->nsector -= n;
>>>     }
>>> 
>>> -    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d sector_num: %ld\n",
>>> +    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d "
>>> +                  "sector_num: %" PRId64 "\n",
>>>                   io->remainder_len, io->len, s->nsector, sector_num);
>>>     if (io->remainder_len && io->len) {
>>>         /* guest wants the rest of its previous transfer */
>>>         int remainder_len = MIN(io->remainder_len, io->len);
>>>         uint8_t *p = &io->remainder[0x200 - remainder_len];
>>> 
>>> -        MACIO_DPRINTF("copying remainder %d bytes at %#lx\n",
>>> +        MACIO_DPRINTF("copying remainder %d bytes at %#" HWADDR_PRIx "\n",
>>>                       remainder_len, io->addr);
>>> 
>>>         switch (s->dma_cmd) {
>>> @@ -261,7 +262,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
>>>     if (unaligned) {
>>>         int nsector = io->len >> 9;
>>> 
>>> -        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
>>> +        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
>>>                       unaligned, io->addr + io->len - unaligned);
>>> 
>>>         switch (s->dma_cmd) {
>> 
>> Ping?
> 
> 
> Ping²?

Acked-by: Alexander Graf <agraf@suse.de>

I assume this can go through the trivial tree? Or directly get applied by Anthony?


Alex
Stefan Weil - July 19, 2013, 6:59 p.m.
Am 19.07.2013 00:40, schrieb Alexander Graf:
> On 18.07.2013, at 21:49, Stefan Weil wrote:
>
>> Am 16.07.2013 07:54, schrieb Stefan Weil:
>>> Am 12.07.2013 18:48, schrieb Stefan Weil:
>>>> Fix a number of warnings for 32 bit builds (tested on MingW and Linux):
>>>>
>>>>  CC    hw/ide/macio.o
>>>> qemu/hw/ide/macio.c: In function 'pmac_ide_atapi_transfer_cb':
>>>> qemu/hw/ide/macio.c:134:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>>>> qemu/hw/ide/macio.c: In function 'pmac_ide_transfer_cb':
>>>> qemu/hw/ide/macio.c:215:5: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'int64_t' [-Werror=format]
>>>> qemu/hw/ide/macio.c:222:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>>>> qemu/hw/ide/macio.c:264:9: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'hwaddr' [-Werror=format]
>>>> cc1: all warnings being treated as errors
>>>> make: *** [hw/ide/macio.o] Error 1
>>>>
>>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>>> ---
>>>>
>>>>
>>>>
>>>> Hi Anthony,
>>>>
>>>> the patch fixes a build regression which was introduced today.
>>>> Could you please apply it without waiting for the next pull requests?
>>>>
>>>> Thanks,
>>>> Stefan

[...]

> Acked-by: Alexander Graf <agraf@suse.de> I assume this can go through
> the trivial tree? Or directly get applied by Anthony? Alex 


I still think that build regressions should be fixed by a direct
maintainer commit instead of waiting for a pull request,
but obviously this does not happen here (why?).

So hopefully the fix will be committed via qemu-trivial at least.
See http://patchwork.ozlabs.org/patch/258774/ for the full patch.

Stefan
Andreas Färber - July 20, 2013, 2:24 p.m.
Am 19.07.2013 20:59, schrieb Stefan Weil:
> Am 19.07.2013 00:40, schrieb Alexander Graf:
>> On 18.07.2013, at 21:49, Stefan Weil wrote:
>>> Am 16.07.2013 07:54, schrieb Stefan Weil:
>>>> Am 12.07.2013 18:48, schrieb Stefan Weil:
>>>>> Hi Anthony,
>>>>>
>>>>> the patch fixes a build regression which was introduced today.
>>>>> Could you please apply it without waiting for the next pull requests?
>>>>>
>>>>> Thanks,
>>>>> Stefan
> [...]
>> Acked-by: Alexander Graf <agraf@suse.de> I assume this can go through
>> the trivial tree? Or directly get applied by Anthony? Alex 
> 
> 
> I still think that build regressions should be fixed by a direct
> maintainer commit instead of waiting for a pull request,
> but obviously this does not happen here (why?).

Anthony recently stated very clearly that in order for him to pick up a
patch it needs to have a Reviewed-by. This one only got an Acked-by.

> So hopefully the fix will be committed via qemu-trivial at least.

It's not obvious from the diff context whether those specifiers are in
fact correct, and there is an active maintainer, so in theory it
wouldn't qualify for -trivial.

Alex, don't be lazy and just send a one-patch PULL like Cornelia did for
her recent mingw breakage. :)

Andreas

> See http://patchwork.ozlabs.org/patch/258774/ for the full patch.
> 
> Stefan
Stefan Weil - July 21, 2013, 2:58 p.m.
Am 20.07.2013 16:24, schrieb Andreas Färber:
>
> Anthony recently stated very clearly that in order for him to pick up a
> patch it needs to have a Reviewed-by. This one only got an Acked-by.


No rule without exception. When I look at the list of patches
which were picked up recently, I see lots of patches without
a Reviewed-by.

I still think that build regressions should be fixed
very fast and don't understand why it takes so long:

http://patchwork.ozlabs.org/patch/257203/ (2013-07-05)
http://patchwork.ozlabs.org/patch/258774/ (2013-07-12)

Do we need more people with commit rights?
Michael Tokarev - July 23, 2013, 5:21 p.m.
21.07.2013 18:58, Stefan Weil wrote:
> Am 20.07.2013 16:24, schrieb Andreas Färber:
>>
>> Anthony recently stated very clearly that in order for him to pick up a
>> patch it needs to have a Reviewed-by. This one only got an Acked-by.
> 
> 
> No rule without exception. When I look at the list of patches
> which were picked up recently, I see lots of patches without
> a Reviewed-by.
> 
> I still think that build regressions should be fixed
> very fast and don't understand why it takes so long:

I reviewed the patch and it appears to be correct.
Applied to the trivial patches queue.
Thanks,

/mjt

Patch

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 38ad924..ef4ba2b 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -131,7 +131,7 @@  static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
         int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
         int nsector = io->len >> 9;
 
-        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
+        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
                       unaligned, io->addr + io->len - unaligned);
 
         bdrv_read(s->bs, sector_num + nsector, io->remainder, 1);
@@ -212,14 +212,15 @@  static void pmac_ide_transfer_cb(void *opaque, int ret)
         s->nsector -= n;
     }
 
-    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d sector_num: %ld\n",
+    MACIO_DPRINTF("remainder: %d io->len: %d nsector: %d "
+                  "sector_num: %" PRId64 "\n",
                   io->remainder_len, io->len, s->nsector, sector_num);
     if (io->remainder_len && io->len) {
         /* guest wants the rest of its previous transfer */
         int remainder_len = MIN(io->remainder_len, io->len);
         uint8_t *p = &io->remainder[0x200 - remainder_len];
 
-        MACIO_DPRINTF("copying remainder %d bytes at %#lx\n",
+        MACIO_DPRINTF("copying remainder %d bytes at %#" HWADDR_PRIx "\n",
                       remainder_len, io->addr);
 
         switch (s->dma_cmd) {
@@ -261,7 +262,7 @@  static void pmac_ide_transfer_cb(void *opaque, int ret)
     if (unaligned) {
         int nsector = io->len >> 9;
 
-        MACIO_DPRINTF("precopying unaligned %d bytes to %#lx\n",
+        MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx "\n",
                       unaligned, io->addr + io->len - unaligned);
 
         switch (s->dma_cmd) {