qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF


From: Mark Cave-Ayland
Subject: Re: [Qemu-trivial] [PATCH] hw/timer/mt48t59: Fix bit-rotten NVRAM_PRINTF format strings
Date: Sat, 3 Feb 2018 10:54:56 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

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 <address@hidden>
---
  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 <address@hidden>


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]