[PATCH-for-5.2 0/4] misc: Trivial format string fixes

[PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Philippe Mathieu-Daudé]

IIUC qemu-trivial@ doesn't queue patches during freeze,
so it might be easier if patches are queued by respective
subsystem maintainers.

Philippe Mathieu-Daudé (4):
  hw/display/cirrus_vga: Remove debugging code commented out
  hw/display/cirrus_vga: Fix hexadecimal format string specifier
  hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
  migration/ram: Fix hexadecimal format string specifier

 hw/display/cirrus_vga.c | 20 +-------------------
 migration/ram.c         |  2 +-
 hw/ppc/trace-events     |  2 +-
 3 files changed, 3 insertions(+), 21 deletions(-)

-- 
2.26.2

[PATCH-for-5.2 1/4] hw/display/cirrus_vga: Remove debugging code commented out

[Philippe Mathieu-Daudé]

Commit ec87f206d70 ("cirrus: replace debug printf with trace points")
forgot to remove this code once replaced. Do it now.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/display/cirrus_vga.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 722b9e7004c..e14096deb46 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2532,9 +2532,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr,
 	case 0x3c5:
 	    val = cirrus_vga_read_sr(c);
             break;
-#ifdef DEBUG_VGA_REG
-	    printf("vga: read SR%x = 0x%02x\n", s->sr_index, val);
-#endif
 	    break;
 	case 0x3c6:
 	    val = cirrus_read_hidden_dac(c);
@@ -2560,9 +2557,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr,
 	    break;
 	case 0x3cf:
 	    val = cirrus_vga_read_gr(c, s->gr_index);
-#ifdef DEBUG_VGA_REG
-	    printf("vga: read GR%x = 0x%02x\n", s->gr_index, val);
-#endif
 	    break;
 	case 0x3b4:
 	case 0x3d4:
@@ -2571,9 +2565,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr,
 	case 0x3b5:
 	case 0x3d5:
             val = cirrus_vga_read_cr(c, s->cr_index);
-#ifdef DEBUG_VGA_REG
-	    printf("vga: read CR%x = 0x%02x\n", s->cr_index, val);
-#endif
 	    break;
 	case 0x3ba:
 	case 0x3da:
@@ -2645,9 +2636,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
 	s->sr_index = val;
 	break;
     case 0x3c5:
-#ifdef DEBUG_VGA_REG
-	printf("vga: write SR%x = 0x%02" PRIu64 "\n", s->sr_index, val);
-#endif
 	cirrus_vga_write_sr(c, val);
         break;
     case 0x3c6:
@@ -2670,9 +2658,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
 	s->gr_index = val;
 	break;
     case 0x3cf:
-#ifdef DEBUG_VGA_REG
-	printf("vga: write GR%x = 0x%02" PRIu64 "\n", s->gr_index, val);
-#endif
 	cirrus_vga_write_gr(c, s->gr_index, val);
 	break;
     case 0x3b4:
@@ -2681,9 +2666,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
 	break;
     case 0x3b5:
     case 0x3d5:
-#ifdef DEBUG_VGA_REG
-	printf("vga: write CR%x = 0x%02"PRIu64"\n", s->cr_index, val);
-#endif
 	cirrus_vga_write_cr(c, val);
 	break;
     case 0x3ba:
-- 
2.26.2

[PATCH-for-5.2 2/4] hw/display/cirrus_vga: Fix hexadecimal format string specifier

[Philippe Mathieu-Daudé]

The '%u' conversion specifier is for decimal notation.
When prefixing a format with '0x', we want the hexadecimal
specifier ('%x').

Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/display/cirrus_vga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index e14096deb46..fdca6ca659f 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2105,7 +2105,7 @@ static void cirrus_vga_mem_write(void *opaque,
     } else {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "cirrus: mem_writeb 0x" TARGET_FMT_plx " "
-                      "value 0x%02" PRIu64 "\n", addr, mem_value);
+                      "value 0x%02" PRIx64 "\n", addr, mem_value);
     }
 }
 
-- 
2.26.2

[PATCH-for-5.2 3/4] hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier

[Philippe Mathieu-Daudé]

The '%u' conversion specifier is for decimal notation.
When prefixing a format with '0x', we want the hexadecimal
specifier ('%x').

Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ppc/trace-events | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index dcc06d49b5a..6d8d095aa28 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -19,7 +19,7 @@ spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old
 spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
 
 # spapr_tpm_proxy.c
-spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIu64
+spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIx64
 spapr_tpm_execute(uint64_t data_in, uint64_t data_in_sz, uint64_t data_out, uint64_t data_out_sz) "data_in=0x%"PRIx64", data_in_sz=%"PRIu64", data_out=0x%"PRIx64", data_out_sz=%"PRIu64
 
 # spapr_iommu.c
-- 
2.26.2

[PATCH-for-5.2 4/4] migration/ram: Fix hexadecimal format string specifier

[Philippe Mathieu-Daudé]

The '%u' conversion specifier is for decimal notation.
When prefixing a format with '0x', we want the hexadecimal
specifier ('%x').

Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 migration/ram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 2da2b622ab2..23dcfb3ac38 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3729,7 +3729,7 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
     }
 
     if (end_mark != RAMBLOCK_RECV_BITMAP_ENDING) {
-        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIu64,
+        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIx64,
                      __func__, block->idstr, end_mark);
         ret = -EINVAL;
         goto out;
-- 
2.26.2

Re: [PATCH-for-5.2 3/4] hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

On Tue, Nov 03, 2020 at 12:25:57PM +0100, Philippe Mathieu-Daudé wrote:
> The '%u' conversion specifier is for decimal notation.
> When prefixing a format with '0x', we want the hexadecimal
> specifier ('%x').
> 
> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/trace-events | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index dcc06d49b5a..6d8d095aa28 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -19,7 +19,7 @@ spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old
>  spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
>  
>  # spapr_tpm_proxy.c
> -spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIu64
> +spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIx64
>  spapr_tpm_execute(uint64_t data_in, uint64_t data_in_sz, uint64_t data_out, uint64_t data_out_sz) "data_in=0x%"PRIx64", data_in_sz=%"PRIu64", data_out=0x%"PRIx64", data_out_sz=%"PRIu64
>  
>  # spapr_iommu.c

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH-for-5.2 3/4] hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier

[Greg Kurz]

On Tue,  3 Nov 2020 12:25:57 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> The '%u' conversion specifier is for decimal notation.
> When prefixing a format with '0x', we want the hexadecimal
> specifier ('%x').
> 
> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/trace-events | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index dcc06d49b5a..6d8d095aa28 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -19,7 +19,7 @@ spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old
>  spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
>  
>  # spapr_tpm_proxy.c
> -spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIu64
> +spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIx64
>  spapr_tpm_execute(uint64_t data_in, uint64_t data_in_sz, uint64_t data_out, uint64_t data_out_sz) "data_in=0x%"PRIx64", data_in_sz=%"PRIu64", data_out=0x%"PRIx64", data_out_sz=%"PRIu64
>  
>  # spapr_iommu.c

Re: [PATCH-for-5.2 1/4] hw/display/cirrus_vga: Remove debugging code commented out

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Commit ec87f206d70 ("cirrus: replace debug printf with trace points")
> forgot to remove this code once replaced. Do it now.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  hw/display/cirrus_vga.c | 18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 722b9e7004c..e14096deb46 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2532,9 +2532,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr,
>  	case 0x3c5:
>  	    val = cirrus_vga_read_sr(c);
>              break;
> -#ifdef DEBUG_VGA_REG
> -	    printf("vga: read SR%x = 0x%02x\n", s->sr_index, val);
> -#endif
>  	    break;
>  	case 0x3c6:
>  	    val = cirrus_read_hidden_dac(c);
> @@ -2560,9 +2557,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr,
>  	    break;
>  	case 0x3cf:
>  	    val = cirrus_vga_read_gr(c, s->gr_index);
> -#ifdef DEBUG_VGA_REG
> -	    printf("vga: read GR%x = 0x%02x\n", s->gr_index, val);
> -#endif
>  	    break;
>  	case 0x3b4:
>  	case 0x3d4:
> @@ -2571,9 +2565,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr,
>  	case 0x3b5:
>  	case 0x3d5:
>              val = cirrus_vga_read_cr(c, s->cr_index);
> -#ifdef DEBUG_VGA_REG
> -	    printf("vga: read CR%x = 0x%02x\n", s->cr_index, val);
> -#endif
>  	    break;
>  	case 0x3ba:
>  	case 0x3da:
> @@ -2645,9 +2636,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
>  	s->sr_index = val;
>  	break;
>      case 0x3c5:
> -#ifdef DEBUG_VGA_REG
> -	printf("vga: write SR%x = 0x%02" PRIu64 "\n", s->sr_index, val);
> -#endif
>  	cirrus_vga_write_sr(c, val);
>          break;
>      case 0x3c6:
> @@ -2670,9 +2658,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
>  	s->gr_index = val;
>  	break;
>      case 0x3cf:
> -#ifdef DEBUG_VGA_REG
> -	printf("vga: write GR%x = 0x%02" PRIu64 "\n", s->gr_index, val);
> -#endif
>  	cirrus_vga_write_gr(c, s->gr_index, val);
>  	break;
>      case 0x3b4:
> @@ -2681,9 +2666,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
>  	break;
>      case 0x3b5:
>      case 0x3d5:
> -#ifdef DEBUG_VGA_REG
> -	printf("vga: write CR%x = 0x%02"PRIu64"\n", s->cr_index, val);
> -#endif
>  	cirrus_vga_write_cr(c, val);
>  	break;
>      case 0x3ba:
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH-for-5.2 2/4] hw/display/cirrus_vga: Fix hexadecimal format string specifier

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> The '%u' conversion specifier is for decimal notation.
> When prefixing a format with '0x', we want the hexadecimal
> specifier ('%x').
> 
> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Oh that's nice, one my regex wouldn't find.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  hw/display/cirrus_vga.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index e14096deb46..fdca6ca659f 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2105,7 +2105,7 @@ static void cirrus_vga_mem_write(void *opaque,
>      } else {
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "cirrus: mem_writeb 0x" TARGET_FMT_plx " "
> -                      "value 0x%02" PRIu64 "\n", addr, mem_value);
> +                      "value 0x%02" PRIx64 "\n", addr, mem_value);
>      }
>  }
>  
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH-for-5.2 4/4] migration/ram: Fix hexadecimal format string specifier

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> The '%u' conversion specifier is for decimal notation.
> When prefixing a format with '0x', we want the hexadecimal
> specifier ('%x').
> 
> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/ram.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 2da2b622ab2..23dcfb3ac38 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3729,7 +3729,7 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
>      }
>  
>      if (end_mark != RAMBLOCK_RECV_BITMAP_ENDING) {
> -        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIu64,
> +        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIx64,
>                       __func__, block->idstr, end_mark);
>          ret = -EINVAL;
>          goto out;
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Dov Murik]

On 03/11/2020 13:25, Philippe Mathieu-Daudé wrote:
> IIUC qemu-trivial@ doesn't queue patches during freeze,
> 
> so it might be easier if patches are queued by respective
> 
> subsystem maintainers.
> 
> 
> 
> Philippe Mathieu-Daudé (4):
> 
>    hw/display/cirrus_vga: Remove debugging code commented out
> 
>    hw/display/cirrus_vga: Fix hexadecimal format string specifier
> 
>    hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
> 
>    migration/ram: Fix hexadecimal format string specifier
> 
> 
> 
>   hw/display/cirrus_vga.c | 20 +-------------------
> 
>   migration/ram.c         |  2 +-
> 
>   hw/ppc/trace-events     |  2 +-
> 
>   3 files changed, 3 insertions(+), 21 deletions(-)
> 
> 
> 


There's at least one more easy fix:

hw/misc/trace-events:106:mos6522_get_next_irq_time(uint16_t latch, 
int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " 
delta_next=0x%"PRId64

but I have no idea how to test this.

-Dov

Re: [PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Dov Murik]

On 03/11/2020 15:58, Dov Murik wrote:
> On 03/11/2020 13:25, Philippe Mathieu-Daudé wrote:
>> IIUC qemu-trivial@ doesn't queue patches during freeze,
>>
>> so it might be easier if patches are queued by respective
>>
>> subsystem maintainers.
>>
>>
>>
>> Philippe Mathieu-Daudé (4):
>>
>>    hw/display/cirrus_vga: Remove debugging code commented out
>>
>>    hw/display/cirrus_vga: Fix hexadecimal format string specifier
>>
>>    hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
>>
>>    migration/ram: Fix hexadecimal format string specifier
>>
>>
>>
>>   hw/display/cirrus_vga.c | 20 +-------------------
>>
>>   migration/ram.c         |  2 +-
>>
>>   hw/ppc/trace-events     |  2 +-
>>
>>   3 files changed, 3 insertions(+), 21 deletions(-)
>>
>>
>>
> 
> 
> There's at least one more easy fix:
> 
> hw/misc/trace-events:106:mos6522_get_next_irq_time(uint16_t latch, 
> int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " 
> delta_next=0x%"PRId64
> 
> but I have no idea how to test this.
> 
> -Dov
> 

... and one more in hw/usb/u2f-passthru.c:348 :

         error_report("%s: Bad written size (req 0x%zu, val 0x%zd)",
                      TYPE_U2F_PASSTHRU, sizeof(host_packet), written);


Again, I have no idea how to test/trigger these areas in the code.

-Dov

Re: [PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Philippe Mathieu-Daudé]

On 11/3/20 3:03 PM, Dov Murik wrote:
> 
> On 03/11/2020 15:58, Dov Murik wrote:
>> On 03/11/2020 13:25, Philippe Mathieu-Daudé wrote:
>>> IIUC qemu-trivial@ doesn't queue patches during freeze,
>>>
>>> so it might be easier if patches are queued by respective
>>>
>>> subsystem maintainers.
>>>
>>>
>>>
>>> Philippe Mathieu-Daudé (4):
>>>
>>>    hw/display/cirrus_vga: Remove debugging code commented out
>>>
>>>    hw/display/cirrus_vga: Fix hexadecimal format string specifier
>>>
>>>    hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
>>>
>>>    migration/ram: Fix hexadecimal format string specifier
>>>
>>>
>>>
>>>   hw/display/cirrus_vga.c | 20 +-------------------
>>>
>>>   migration/ram.c         |  2 +-
>>>
>>>   hw/ppc/trace-events     |  2 +-
>>>
>>>   3 files changed, 3 insertions(+), 21 deletions(-)
>>>
>>>
>>>
>>
>>
>> There's at least one more easy fix:
>>
>> hw/misc/trace-events:106:mos6522_get_next_irq_time(uint16_t latch,
>> int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 "
>> delta_next=0x%"PRId64

Indeed.

>>
>> but I have no idea how to test this.
>>
>> -Dov
>>
> 
> ... and one more in hw/usb/u2f-passthru.c:348 :
> 
>         error_report("%s: Bad written size (req 0x%zu, val 0x%zd)",
>                      TYPE_U2F_PASSTHRU, sizeof(host_packet), written);

'written' is signed, so this format looks correct...

> 
> Again, I have no idea how to test/trigger these areas in the code.
> 
> -Dov
> 

Re: [PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Greg Kurz]

On Tue, 3 Nov 2020 15:28:11 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 11/3/20 3:03 PM, Dov Murik wrote:
> > 
> > On 03/11/2020 15:58, Dov Murik wrote:
> >> On 03/11/2020 13:25, Philippe Mathieu-Daudé wrote:
> >>> IIUC qemu-trivial@ doesn't queue patches during freeze,
> >>>
> >>> so it might be easier if patches are queued by respective
> >>>
> >>> subsystem maintainers.
> >>>
> >>>
> >>>
> >>> Philippe Mathieu-Daudé (4):
> >>>
> >>>    hw/display/cirrus_vga: Remove debugging code commented out
> >>>
> >>>    hw/display/cirrus_vga: Fix hexadecimal format string specifier
> >>>
> >>>    hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
> >>>
> >>>    migration/ram: Fix hexadecimal format string specifier
> >>>
> >>>
> >>>
> >>>   hw/display/cirrus_vga.c | 20 +-------------------
> >>>
> >>>   migration/ram.c         |  2 +-
> >>>
> >>>   hw/ppc/trace-events     |  2 +-
> >>>
> >>>   3 files changed, 3 insertions(+), 21 deletions(-)
> >>>
> >>>
> >>>
> >>
> >>
> >> There's at least one more easy fix:
> >>
> >> hw/misc/trace-events:106:mos6522_get_next_irq_time(uint16_t latch,
> >> int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 "
> >> delta_next=0x%"PRId64
> 
> Indeed.
> 
> >>
> >> but I have no idea how to test this.
> >>
> >> -Dov
> >>
> > 
> > ... and one more in hw/usb/u2f-passthru.c:348 :
> > 
> >         error_report("%s: Bad written size (req 0x%zu, val 0x%zd)",
> >                      TYPE_U2F_PASSTHRU, sizeof(host_packet), written);
> 
> 'written' is signed, so this format looks correct...
> 

Irrespective of the sign, u and d shouldn't be used behind 0x 

> > 
> > Again, I have no idea how to test/trigger these areas in the code.
> > 
> > -Dov
> > 
> 
> 

Re: [PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Dov Murik]

On 03/11/2020 16:28, Philippe Mathieu-Daudé wrote:
> On 11/3/20 3:03 PM, Dov Murik wrote:
>>
>> On 03/11/2020 15:58, Dov Murik wrote:
>>> On 03/11/2020 13:25, Philippe Mathieu-Daudé wrote:
>>>> IIUC qemu-trivial@ doesn't queue patches during freeze,
>>>>
>>>> so it might be easier if patches are queued by respective
>>>>
>>>> subsystem maintainers.
>>>>
>>>>
>>>>
>>>> Philippe Mathieu-Daudé (4):
>>>>
>>>>     hw/display/cirrus_vga: Remove debugging code commented out
>>>>
>>>>     hw/display/cirrus_vga: Fix hexadecimal format string specifier
>>>>
>>>>     hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
>>>>
>>>>     migration/ram: Fix hexadecimal format string specifier
>>>>
>>>>
>>>>
>>>>    hw/display/cirrus_vga.c | 20 +-------------------
>>>>
>>>>    migration/ram.c         |  2 +-
>>>>
>>>>    hw/ppc/trace-events     |  2 +-
>>>>
>>>>    3 files changed, 3 insertions(+), 21 deletions(-)
>>>>
>>>>
>>>>
>>>
>>>
>>> There's at least one more easy fix:
>>>
>>> hw/misc/trace-events:106:mos6522_get_next_irq_time(uint16_t latch,
>>> int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 "
>>> delta_next=0x%"PRId64
> 
> Indeed.
> 
>>>
>>> but I have no idea how to test this.
>>>
>>> -Dov
>>>
>>
>> ... and one more in hw/usb/u2f-passthru.c:348 :
>>
>>          error_report("%s: Bad written size (req 0x%zu, val 0x%zd)",
>>                       TYPE_U2F_PASSTHRU, sizeof(host_packet), written);
> 
> 'written' is signed, so this format looks correct...
> 

If we keep the %zu and %zd format specifiers, then I suggest removing 
the "0x" literal prefixes.


>>
>> Again, I have no idea how to test/trigger these areas in the code.
>>
>> -Dov
>>
> 
> 

Re: [PATCH-for-5.2 0/4] misc: Trivial format string fixes

[Philippe Mathieu-Daudé]

On 11/3/20 4:20 PM, Greg Kurz wrote:
> On Tue, 3 Nov 2020 15:28:11 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 11/3/20 3:03 PM, Dov Murik wrote:
>>>
>>> On 03/11/2020 15:58, Dov Murik wrote:
>>>> On 03/11/2020 13:25, Philippe Mathieu-Daudé wrote:
>>>>> IIUC qemu-trivial@ doesn't queue patches during freeze,
>>>>>
>>>>> so it might be easier if patches are queued by respective
>>>>>
>>>>> subsystem maintainers.
>>>>>
>>>>>
>>>>>
>>>>> Philippe Mathieu-Daudé (4):
>>>>>
>>>>>    hw/display/cirrus_vga: Remove debugging code commented out
>>>>>
>>>>>    hw/display/cirrus_vga: Fix hexadecimal format string specifier
>>>>>
>>>>>    hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier
>>>>>
>>>>>    migration/ram: Fix hexadecimal format string specifier
>>>>>
>>>>>
>>>>>
>>>>>   hw/display/cirrus_vga.c | 20 +-------------------
>>>>>
>>>>>   migration/ram.c         |  2 +-
>>>>>
>>>>>   hw/ppc/trace-events     |  2 +-
>>>>>
>>>>>   3 files changed, 3 insertions(+), 21 deletions(-)
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> There's at least one more easy fix:
>>>>
>>>> hw/misc/trace-events:106:mos6522_get_next_irq_time(uint16_t latch,
>>>> int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 "
>>>> delta_next=0x%"PRId64
>>
>> Indeed.
>>
>>>>
>>>> but I have no idea how to test this.
>>>>
>>>> -Dov
>>>>
>>>
>>> ... and one more in hw/usb/u2f-passthru.c:348 :
>>>
>>>         error_report("%s: Bad written size (req 0x%zu, val 0x%zd)",
>>>                      TYPE_U2F_PASSTHRU, sizeof(host_packet), written);
>>
>> 'written' is signed, so this format looks correct...
>>
> 
> Irrespective of the sign, u and d shouldn't be used behind 0x 

Ah yes you are right =) Long day... So '0x%zx' in both cases.

> 
>>>
>>> Again, I have no idea how to test/trigger these areas in the code.
>>>
>>> -Dov
>>>
>>
>>
> 

Re: [PATCH-for-5.2 3/4] hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier

[Philippe Mathieu-Daudé]

Hi David,

On 11/3/20 12:39 PM, David Gibson wrote:
> On Tue, Nov 03, 2020 at 12:25:57PM +0100, Philippe Mathieu-Daudé wrote:
>> The '%u' conversion specifier is for decimal notation.
>> When prefixing a format with '0x', we want the hexadecimal
>> specifier ('%x').
>>
>> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

As there is no qemu-trivial@ pull request during freeze/rc,
can you queue this patch via your ppc tree?

Thanks,

Phil.

> 
>> ---
>>  hw/ppc/trace-events | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
>> index dcc06d49b5a..6d8d095aa28 100644
>> --- a/hw/ppc/trace-events
>> +++ b/hw/ppc/trace-events
>> @@ -19,7 +19,7 @@ spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old
>>  spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
>>  
>>  # spapr_tpm_proxy.c
>> -spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIu64
>> +spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIx64
>>  spapr_tpm_execute(uint64_t data_in, uint64_t data_in_sz, uint64_t data_out, uint64_t data_out_sz) "data_in=0x%"PRIx64", data_in_sz=%"PRIu64", data_out=0x%"PRIx64", data_out_sz=%"PRIu64
>>  
>>  # spapr_iommu.c
> 

Re: [PATCH-for-5.2 2/4] hw/display/cirrus_vga: Fix hexadecimal format string specifier

[Philippe Mathieu-Daudé]

Hi Gerd,

On 11/3/20 12:25 PM, Philippe Mathieu-Daudé wrote:
> The '%u' conversion specifier is for decimal notation.
> When prefixing a format with '0x', we want the hexadecimal
> specifier ('%x').
> 
> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/display/cirrus_vga.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index e14096deb46..fdca6ca659f 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2105,7 +2105,7 @@ static void cirrus_vga_mem_write(void *opaque,
>      } else {
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "cirrus: mem_writeb 0x" TARGET_FMT_plx " "
> -                      "value 0x%02" PRIu64 "\n", addr, mem_value);
> +                      "value 0x%02" PRIx64 "\n", addr, mem_value);
>      }
>  }
>  
> 

This patch (and previous one) missed your latest ui pull
request. As there is no qemu-trivial@ pull request during
freeze/rc, can you queue these patches via your tree?

Thanks,

Phil.

Re: [PATCH-for-5.2 4/4] migration/ram: Fix hexadecimal format string specifier

[Philippe Mathieu-Daudé]

Hi David, Juan.

On 11/3/20 1:46 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> The '%u' conversion specifier is for decimal notation.
>> When prefixing a format with '0x', we want the hexadecimal
>> specifier ('%x').
>>
>> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

As there is no qemu-trivial@ pull request during freeze/rc,
can you queue this patch via your migration tree?

Thanks,

Phil.

> 
>> ---
>>  migration/ram.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 2da2b622ab2..23dcfb3ac38 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -3729,7 +3729,7 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
>>      }
>>  
>>      if (end_mark != RAMBLOCK_RECV_BITMAP_ENDING) {
>> -        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIu64,
>> +        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIx64,
>>                       __func__, block->idstr, end_mark);
>>          ret = -EINVAL;
>>          goto out;
>> -- 
>> 2.26.2
>>

Re: [PATCH-for-5.2 4/4] migration/ram: Fix hexadecimal format string specifier

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Hi David, Juan.
> 
> On 11/3/20 1:46 PM, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> >> The '%u' conversion specifier is for decimal notation.
> >> When prefixing a format with '0x', we want the hexadecimal
> >> specifier ('%x').
> >>
> >> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> As there is no qemu-trivial@ pull request during freeze/rc,
> can you queue this patch via your migration tree?

Yep, will do.

Dave

> Thanks,
> 
> Phil.
> 
> > 
> >> ---
> >>  migration/ram.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index 2da2b622ab2..23dcfb3ac38 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -3729,7 +3729,7 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
> >>      }
> >>  
> >>      if (end_mark != RAMBLOCK_RECV_BITMAP_ENDING) {
> >> -        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIu64,
> >> +        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIx64,
> >>                       __func__, block->idstr, end_mark);
> >>          ret = -EINVAL;
> >>          goto out;
> >> -- 
> >> 2.26.2
> >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH-for-5.2 4/4] migration/ram: Fix hexadecimal format string specifier

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> > Hi David, Juan.
> > 
> > On 11/3/20 1:46 PM, Dr. David Alan Gilbert wrote:
> > > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> > >> The '%u' conversion specifier is for decimal notation.
> > >> When prefixing a format with '0x', we want the hexadecimal
> > >> specifier ('%x').
> > >>
> > >> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > 
> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > 
> > As there is no qemu-trivial@ pull request during freeze/rc,
> > can you queue this patch via your migration tree?
> 
> Yep, will do.

Queued

> Dave
> 
> > Thanks,
> > 
> > Phil.
> > 
> > > 
> > >> ---
> > >>  migration/ram.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/migration/ram.c b/migration/ram.c
> > >> index 2da2b622ab2..23dcfb3ac38 100644
> > >> --- a/migration/ram.c
> > >> +++ b/migration/ram.c
> > >> @@ -3729,7 +3729,7 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
> > >>      }
> > >>  
> > >>      if (end_mark != RAMBLOCK_RECV_BITMAP_ENDING) {
> > >> -        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIu64,
> > >> +        error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIx64,
> > >>                       __func__, block->idstr, end_mark);
> > >>          ret = -EINVAL;
> > >>          goto out;
> > >> -- 
> > >> 2.26.2
> > >>
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH-for-5.2 3/4] hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]

On Mon, Nov 09, 2020 at 03:28:24PM +0100, Philippe Mathieu-Daudé wrote:
> Hi David,
> 
> On 11/3/20 12:39 PM, David Gibson wrote:
> > On Tue, Nov 03, 2020 at 12:25:57PM +0100, Philippe Mathieu-Daudé wrote:
> >> The '%u' conversion specifier is for decimal notation.
> >> When prefixing a format with '0x', we want the hexadecimal
> >> specifier ('%x').
> >>
> >> Inspired-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > 
> > Acked-by: David Gibson <david@gibson.dropbear.id.au>
> 
> As there is no qemu-trivial@ pull request during freeze/rc,
> can you queue this patch via your ppc tree?

Done, applied to ppc-for-6.0.

> 
> Thanks,
> 
> Phil.
> 
> > 
> >> ---
> >>  hw/ppc/trace-events | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> >> index dcc06d49b5a..6d8d095aa28 100644
> >> --- a/hw/ppc/trace-events
> >> +++ b/hw/ppc/trace-events
> >> @@ -19,7 +19,7 @@ spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old
> >>  spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
> >>  
> >>  # spapr_tpm_proxy.c
> >> -spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIu64
> >> +spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIx64
> >>  spapr_tpm_execute(uint64_t data_in, uint64_t data_in_sz, uint64_t data_out, uint64_t data_out_sz) "data_in=0x%"PRIx64", data_in_sz=%"PRIu64", data_out=0x%"PRIx64", data_out_sz=%"PRIu64
> >>  
> >>  # spapr_iommu.c
> > 
> 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]