Subject: [PATCH] kvmppc: kvmtrace: fix TRACE_REC_* macro definitions

Subject: [PATCH] kvmppc: kvmtrace: fix TRACE_REC_* macro definitions

[Christian Ehrhardt]

Subject: [PATCH] kvmppc: kvmtrace: fix TRACE_REC_* macro definitions

From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>

The current patch kvm_trace_nobitfields.diff in the kvmppc.hg-dev repo
doesn't compile.

code:
   rec.rec_val     |= TRACE_REC_EVENT_ID(va_arg(*args, u32));
+makro
   #define TRACE_REC_EVENT_ID (val) \
                (0x0fffffff & (val))

extended to:
   rec.rec_val |= (val) (0x0fffffff & (val))(__builtin_va_arg(*args,u32));

failing with:
   kvm_trace.c: In function 'kvm_add_trace':
   kvm_trace.c:66: error: 'val' undeclared (first use in this function)
   kvm_trace.c:66: error: (Each undeclared identifier is reported only once
   kvm_trace.c:66: error: for each function it appears in.)

Obviously caused by the blank between macro definition and (val) which makes
it a non-parameter macro.

Additionally the two macro definitions below don't put () around the 
parameter
which is one of the golden macro rules to keep operator precedence 
independent
to what someone passes as argument to your macro.

This patch fixes both issues, Hollis could you please fold it into
"kvm_trace_nobitfields.diff" and add my signed-off-by then.

Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

[diffstat]
 kvm.h |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

[diff]

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -331,12 +331,12 @@ struct kvm_trace_rec {
     } u;
 } __attribute__((packed));
 
-#define TRACE_REC_EVENT_ID (val) \
+#define TRACE_REC_EVENT_ID(val) \
         (0x0fffffff & (val))
-#define TRACE_REC_NUM_DATA_ARGS (val) \
-        (0x70000000 & (val << 28))
-#define TRACE_REC_TCS (val) \
-        (0x80000000 & (val << 31))
+#define TRACE_REC_NUM_DATA_ARGS(val) \
+        (0x70000000 & ((val) << 28))
+#define TRACE_REC_TCS(val) \
+        (0x80000000 & ((val) << 31))
 
 #define KVMIO 0xAE
 

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization

Re: Subject: [PATCH] kvmppc: kvmtrace: fix TRACE_REC_* macro

[Hollis Blanchard]

On Wed, 2008-07-02 at 13:55 +0200, Christian Ehrhardt wrote:
> Subject: [PATCH] kvmppc: kvmtrace: fix TRACE_REC_* macro definitions
> 
> From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
> 
> The current patch kvm_trace_nobitfields.diff in the kvmppc.hg-dev repo
> doesn't compile.
> 
> code:
>    rec.rec_val     |= TRACE_REC_EVENT_ID(va_arg(*args, u32));
> +makro
>    #define TRACE_REC_EVENT_ID (val) \
>                 (0x0fffffff & (val))
> 
> extended to:
>    rec.rec_val |= (val) (0x0fffffff & (val))(__builtin_va_arg(*args,u32));
> 
> failing with:
>    kvm_trace.c: In function 'kvm_add_trace':
>    kvm_trace.c:66: error: 'val' undeclared (first use in this function)
>    kvm_trace.c:66: error: (Each undeclared identifier is reported only once
>    kvm_trace.c:66: error: for each function it appears in.)
> 
> Obviously caused by the blank between macro definition and (val) which makes
> it a non-parameter macro.

Hmm, yeah, this is broken.

Avi, you said you applied Jerone's patch #1 (Sun, 29 Jun 2008 14:50:34
+0300 (06:50 CDT)), but I haven't seen it show up on the commits list
yet. Christian is pointing out that it's bad, and needs the fix below.

If you want, I will collect this in my PPC patch queue and send you a
batch soon.

(Christian, feel free to send these non-arch-specific patches directly
to Avi in the future...)

> Additionally the two macro definitions below don't put () around the 
> parameter
> which is one of the golden macro rules to keep operator precedence 
> independent
> to what someone passes as argument to your macro.
> 
> This patch fixes both issues, Hollis could you please fold it into
> "kvm_trace_nobitfields.diff" and add my signed-off-by then.
> 
> Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
> ---
> 
> [diffstat]
>  kvm.h |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> [diff]
> 
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -331,12 +331,12 @@ struct kvm_trace_rec {
>      } u;
>  } __attribute__((packed));
> 
> -#define TRACE_REC_EVENT_ID (val) \
> +#define TRACE_REC_EVENT_ID(val) \
>          (0x0fffffff & (val))
> -#define TRACE_REC_NUM_DATA_ARGS (val) \
> -        (0x70000000 & (val << 28))
> -#define TRACE_REC_TCS (val) \
> -        (0x80000000 & (val << 31))
> +#define TRACE_REC_NUM_DATA_ARGS(val) \
> +        (0x70000000 & ((val) << 28))
> +#define TRACE_REC_TCS(val) \
> +        (0x80000000 & ((val) << 31))
> 
>  #define KVMIO 0xAE
> 
> 
-- 
Hollis Blanchard
IBM Linux Technology Center