| Message ID | 20201103112558.2554390-1-philmd@redhat.com |
|---|---|
| Headers | show |
| Series | misc: Trivial format string fixes | expand |
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
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
* 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
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
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
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 >
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 > > > >
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 >> > >
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 >>> >> >> >
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 >
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.
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