diff mbox

[08/25] monitor: New GET_TLONG and GET_TPHYSADDR macros

Message ID 1249318642-19324-9-git-send-email-lcapitulino@redhat.com
State Superseded
Headers show

Commit Message

Luiz Capitulino Aug. 3, 2009, 4:57 p.m. UTC
When we start porting handlers to use the Monitor's dictionary
to pass argments, we will turn function parameters into automatic
variables.

This will make the build brake when the 32 bits versions of
GET_TLONG and GET_TPHYSADDR are used, because the 'h' argument
will not be used.

The best solution I could think for this problem was changing
both macros to reassign the 'h' parameter when compiled for
32 bits.

I'm open for better solutions, though.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

Comments

Blue Swirl Aug. 4, 2009, 5:27 p.m. UTC | #1
On Mon, Aug 3, 2009 at 7:57 PM, Luiz Capitulino<lcapitulino@redhat.com> wrote:
> When we start porting handlers to use the Monitor's dictionary
> to pass argments, we will turn function parameters into automatic
> variables.
>
> This will make the build brake when the 32 bits versions of
> GET_TLONG and GET_TPHYSADDR are used, because the 'h' argument
> will not be used.
>
> The best solution I could think for this problem was changing
> both macros to reassign the 'h' parameter when compiled for
> 32 bits.
>
> I'm open for better solutions, though.

How about:

#define GET_TLONG(h, l) ((target_ulong)(((uint64_t)(h) << 32) | (l)))
#define GET_TPHYSADDR(h, l) ((target_phys_addr_t)(((uint64_t)(h) << 32) | (l)))

This may introduce new Sparse warnings about truncating cast, but
there are already a lot of those.

I'd use GET_TADDR instead of GET_TPHYSADDR, TADDR is already used by qdev code.
Luiz Capitulino Aug. 4, 2009, 7:42 p.m. UTC | #2
On Tue, 4 Aug 2009 20:27:43 +0300
Blue Swirl <blauwirbel@gmail.com> wrote:

> On Mon, Aug 3, 2009 at 7:57 PM, Luiz Capitulino<lcapitulino@redhat.com> wrote:
> > When we start porting handlers to use the Monitor's dictionary
> > to pass argments, we will turn function parameters into automatic
> > variables.
> >
> > This will make the build brake when the 32 bits versions of
> > GET_TLONG and GET_TPHYSADDR are used, because the 'h' argument
> > will not be used.
> >
> > The best solution I could think for this problem was changing
> > both macros to reassign the 'h' parameter when compiled for
> > 32 bits.
> >
> > I'm open for better solutions, though.
> 
> How about:
> 
> #define GET_TLONG(h, l) ((target_ulong)(((uint64_t)(h) << 32) | (l)))
> #define GET_TPHYSADDR(h, l) ((target_phys_addr_t)(((uint64_t)(h) << 32) | (l)))

 Yes, it will work I think.

 Would you mind waiting for it to get merged to be changed? I
wouldn't like to patchbomb the list because of this change.

> This may introduce new Sparse warnings about truncating cast, but
> there are already a lot of those.
> 
> I'd use GET_TADDR instead of GET_TPHYSADDR, TADDR is already used by qdev code.

 Ok, will have a look, but this kind of change is not part of this
series.
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 676ce7d..8127aa2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -778,7 +778,11 @@  static void memory_dump(Monitor *mon, int count, int format, int wsize,
 #if TARGET_LONG_BITS == 64
 #define GET_TLONG(h, l) (((uint64_t)(h) << 32) | (l))
 #else
-#define GET_TLONG(h, l) (l)
+static inline uint32_t GET_TLONG(uint32_t h, uint32_t l)
+{
+    h = h; /* avoid build error */
+    return l;
+}
 #endif
 
 static void do_memory_dump(Monitor *mon, int count, int format, int size,
@@ -791,7 +795,11 @@  static void do_memory_dump(Monitor *mon, int count, int format, int size,
 #if TARGET_PHYS_ADDR_BITS > 32
 #define GET_TPHYSADDR(h, l) (((uint64_t)(h) << 32) | (l))
 #else
-#define GET_TPHYSADDR(h, l) (l)
+static inline uint32_t GET_TPHYSADDR(uint32_t h, uint32_t l)
+{
+    h = h; /* avoid build error */
+    return l;
+}
 #endif
 
 static void do_physical_memory_dump(Monitor *mon, int count, int format,