* [Qemu-devel] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings
@ 2018-02-02 8:15 Thomas Huth
2018-02-02 20:30 ` Philippe Mathieu-Daudé
2018-02-03 10:54 ` Mark Cave-Ayland
0 siblings, 2 replies; 3+ messages in thread
From: Thomas Huth @ 2018-02-02 8:15 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Mark Cave-Ayland
When compiling with NVRAM_PRINTF enabled, gcc currently bails out with:
CC hw/timer/m48t59.o
CC hw/timer/m48t59-isa.o
hw/timer/m48t59.c: In function ‘NVRAM_writeb’:
hw/timer/m48t59.c:460:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘hwaddr’ [-Werror=format=]
NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
^
hw/timer/m48t59.c:460:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
hw/timer/m48t59.c: In function ‘NVRAM_readb’:
hw/timer/m48t59.c:492:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘hwaddr’ [-Werror=format=]
NVRAM_PRINTF("%s: 0x%08x <= 0x%08x\n", __func__, addr, retval);
Fix it by using the correct format strings and while we're at it,
also change the definition of NVRAM_PRINTF so that this can not
bit-rot so easily again.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/timer/m48t59-internal.h | 9 +++------
hw/timer/m48t59.c | 4 ++--
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
index 32ae957..d0f0caf 100644
--- a/hw/timer/m48t59-internal.h
+++ b/hw/timer/m48t59-internal.h
@@ -25,13 +25,10 @@
#ifndef HW_M48T59_INTERNAL_H
#define HW_M48T59_INTERNAL_H 1
-//#define DEBUG_NVRAM
+#define M48T59_DEBUG 0
-#if defined(DEBUG_NVRAM)
-#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
-#else
-#define NVRAM_PRINTF(fmt, ...) do { } while (0)
-#endif
+#define NVRAM_PRINTF(fmt, ...) do { \
+ if (M48T59_DEBUG) { printf(fmt , ## __VA_ARGS__); } } while (0)
/*
* The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index 844aad5..4abb4ac 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -457,7 +457,7 @@ static void NVRAM_writeb(void *opaque, hwaddr addr, uint64_t val,
{
M48t59State *NVRAM = opaque;
- NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
+ NVRAM_PRINTF("%s: 0x%"HWADDR_PRIx" => 0x%"PRIx64"\n", __func__, addr, val);
switch (addr) {
case 0:
NVRAM->addr &= ~0x00FF;
@@ -489,7 +489,7 @@ static uint64_t NVRAM_readb(void *opaque, hwaddr addr, unsigned size)
retval = -1;
break;
}
- NVRAM_PRINTF("%s: 0x%08x <= 0x%08x\n", __func__, addr, retval);
+ NVRAM_PRINTF("%s: 0x%"HWADDR_PRIx" <= 0x%08x\n", __func__, addr, retval);
return retval;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings
2018-02-02 8:15 [Qemu-devel] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings Thomas Huth
@ 2018-02-02 20:30 ` Philippe Mathieu-Daudé
2018-02-03 10:54 ` Mark Cave-Ayland
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-02 20:30 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: qemu-trivial, Mark Cave-Ayland
Hi Thomas,
On 02/02/2018 05:15 AM, Thomas Huth wrote:
> When compiling with NVRAM_PRINTF enabled, gcc currently bails out with:
>
> CC hw/timer/m48t59.o
> CC hw/timer/m48t59-isa.o
> hw/timer/m48t59.c: In function ‘NVRAM_writeb’:
> hw/timer/m48t59.c:460:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘hwaddr’ [-Werror=format=]
> NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
> ^
> hw/timer/m48t59.c:460:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
> hw/timer/m48t59.c: In function ‘NVRAM_readb’:
> hw/timer/m48t59.c:492:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘hwaddr’ [-Werror=format=]
> NVRAM_PRINTF("%s: 0x%08x <= 0x%08x\n", __func__, addr, retval);
>
> Fix it by using the correct format strings and while we're at it,
> also change the definition of NVRAM_PRINTF so that this can not
> bit-rot so easily again.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/timer/m48t59-internal.h | 9 +++------
> hw/timer/m48t59.c | 4 ++--
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
> index 32ae957..d0f0caf 100644
> --- a/hw/timer/m48t59-internal.h
> +++ b/hw/timer/m48t59-internal.h
> @@ -25,13 +25,10 @@
> #ifndef HW_M48T59_INTERNAL_H
> #define HW_M48T59_INTERNAL_H 1
>
> -//#define DEBUG_NVRAM
> +#define M48T59_DEBUG 0
>
> -#if defined(DEBUG_NVRAM)
> -#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define NVRAM_PRINTF(fmt, ...) do { } while (0)
> -#endif
> +#define NVRAM_PRINTF(fmt, ...) do { \
> + if (M48T59_DEBUG) { printf(fmt , ## __VA_ARGS__); } } while (0)
While not use tracepoints directly?
>
> /*
> * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
> index 844aad5..4abb4ac 100644
> --- a/hw/timer/m48t59.c
> +++ b/hw/timer/m48t59.c
> @@ -457,7 +457,7 @@ static void NVRAM_writeb(void *opaque, hwaddr addr, uint64_t val,
> {
> M48t59State *NVRAM = opaque;
>
> - NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
> + NVRAM_PRINTF("%s: 0x%"HWADDR_PRIx" => 0x%"PRIx64"\n", __func__, addr, val);
> switch (addr) {
> case 0:
> NVRAM->addr &= ~0x00FF;
> @@ -489,7 +489,7 @@ static uint64_t NVRAM_readb(void *opaque, hwaddr addr, unsigned size)
> retval = -1;
> break;
> }
> - NVRAM_PRINTF("%s: 0x%08x <= 0x%08x\n", __func__, addr, retval);
> + NVRAM_PRINTF("%s: 0x%"HWADDR_PRIx" <= 0x%08x\n", __func__, addr, retval);
>
> return retval;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings
2018-02-02 8:15 [Qemu-devel] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings Thomas Huth
2018-02-02 20:30 ` Philippe Mathieu-Daudé
@ 2018-02-03 10:54 ` Mark Cave-Ayland
1 sibling, 0 replies; 3+ messages in thread
From: Mark Cave-Ayland @ 2018-02-03 10:54 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: qemu-trivial
On 02/02/18 08:15, Thomas Huth wrote:
> When compiling with NVRAM_PRINTF enabled, gcc currently bails out with:
>
> CC hw/timer/m48t59.o
> CC hw/timer/m48t59-isa.o
> hw/timer/m48t59.c: In function ‘NVRAM_writeb’:
> hw/timer/m48t59.c:460:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘hwaddr’ [-Werror=format=]
> NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
> ^
> hw/timer/m48t59.c:460:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
> hw/timer/m48t59.c: In function ‘NVRAM_readb’:
> hw/timer/m48t59.c:492:5: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘hwaddr’ [-Werror=format=]
> NVRAM_PRINTF("%s: 0x%08x <= 0x%08x\n", __func__, addr, retval);
>
> Fix it by using the correct format strings and while we're at it,
> also change the definition of NVRAM_PRINTF so that this can not
> bit-rot so easily again.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/timer/m48t59-internal.h | 9 +++------
> hw/timer/m48t59.c | 4 ++--
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
> index 32ae957..d0f0caf 100644
> --- a/hw/timer/m48t59-internal.h
> +++ b/hw/timer/m48t59-internal.h
> @@ -25,13 +25,10 @@
> #ifndef HW_M48T59_INTERNAL_H
> #define HW_M48T59_INTERNAL_H 1
>
> -//#define DEBUG_NVRAM
> +#define M48T59_DEBUG 0
>
> -#if defined(DEBUG_NVRAM)
> -#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define NVRAM_PRINTF(fmt, ...) do { } while (0)
> -#endif
> +#define NVRAM_PRINTF(fmt, ...) do { \
> + if (M48T59_DEBUG) { printf(fmt , ## __VA_ARGS__); } } while (0)
>
> /*
> * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
> index 844aad5..4abb4ac 100644
> --- a/hw/timer/m48t59.c
> +++ b/hw/timer/m48t59.c
> @@ -457,7 +457,7 @@ static void NVRAM_writeb(void *opaque, hwaddr addr, uint64_t val,
> {
> M48t59State *NVRAM = opaque;
>
> - NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
> + NVRAM_PRINTF("%s: 0x%"HWADDR_PRIx" => 0x%"PRIx64"\n", __func__, addr, val);
> switch (addr) {
> case 0:
> NVRAM->addr &= ~0x00FF;
> @@ -489,7 +489,7 @@ static uint64_t NVRAM_readb(void *opaque, hwaddr addr, unsigned size)
> retval = -1;
> break;
> }
> - NVRAM_PRINTF("%s: 0x%08x <= 0x%08x\n", __func__, addr, retval);
> + NVRAM_PRINTF("%s: 0x%"HWADDR_PRIx" <= 0x%08x\n", __func__, addr, retval);
>
> return retval;
> }
Hi Thomas,
I agree with Philippe here that given you've got this far then is it not
worth going all the way with a trace-events conversion?
Then again fixing this is enough of an improvement that I'm happy to
give my R-B regardless:
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-03 10:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-02 8:15 [Qemu-devel] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings Thomas Huth
2018-02-02 20:30 ` Philippe Mathieu-Daudé
2018-02-03 10:54 ` Mark Cave-Ayland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).