* [Qemu-devel] [PATCH target-arm v2] display: avoid multi-statement macro
@ 2014-01-24 19:08 Paolo Bonzini
2014-01-27 14:56 ` Laszlo Ersek
2014-01-27 17:58 ` Peter Maydell
0 siblings, 2 replies; 3+ messages in thread
From: Paolo Bonzini @ 2014-01-24 19:08 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
For blizzard, pl110 and tc6393xb this is harmless, but for pxa2xx
Coverity noticed that it is used inside an "if" statement.
Fix it because it's the file with the highest number of defects
in the whole QEMU tree! Use "do...while(0)", or just remove the
semicolon if there's a single statement in the macro.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/display/blizzard_template.h | 22 ++++++++++++++++------
hw/display/pl110_template.h | 12 ++++++++----
hw/display/pxa2xx_template.h | 22 +++++++++++++++++-----
hw/display/tc6393xb_template.h | 14 +++++++++-----
4 files changed, 56 insertions(+), 27 deletions(-)
diff --git a/hw/display/blizzard_template.h b/hw/display/blizzard_template.h
index a8a8899..c48a9b3 100644
--- a/hw/display/blizzard_template.h
+++ b/hw/display/blizzard_template.h
@@ -21,21 +21,31 @@
#define SKIP_PIXEL(to) to += deststep
#if DEPTH == 8
# define PIXEL_TYPE uint8_t
-# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
# define COPY_PIXEL1(to, from) *to ++ = from
#elif DEPTH == 15 || DEPTH == 16
# define PIXEL_TYPE uint16_t
-# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
# define COPY_PIXEL1(to, from) *to ++ = from
#elif DEPTH == 24
# define PIXEL_TYPE uint8_t
-# define COPY_PIXEL(to, from) \
- to[0] = from; to[1] = (from) >> 8; to[2] = (from) >> 16; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) \
+ do { \
+ to[0] = from; \
+ to[1] = (from) >> 8; \
+ to[2] = (from) >> 16; \
+ SKIP_PIXEL(to); \
+ } while(0)
+
# define COPY_PIXEL1(to, from) \
- *to ++ = from; *to ++ = (from) >> 8; *to ++ = (from) >> 16
+ do { \
+ *to ++ = from; \
+ *to ++ = (from) >> 8; \
+ *to ++ = (from) >> 16; \
+ } while(0)
#elif DEPTH == 32
# define PIXEL_TYPE uint32_t
-# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
# define COPY_PIXEL1(to, from) *to ++ = from
#else
# error unknown bit depth
diff --git a/hw/display/pl110_template.h b/hw/display/pl110_template.h
index e738e4a..f685b26 100644
--- a/hw/display/pl110_template.h
+++ b/hw/display/pl110_template.h
@@ -14,12 +14,16 @@
#if BITS == 8
#define COPY_PIXEL(to, from) *(to++) = from
#elif BITS == 15 || BITS == 16
-#define COPY_PIXEL(to, from) *(uint16_t *)to = from; to += 2;
+#define COPY_PIXEL(to, from) do { *(uint16_t *)to = from; to += 2; } while(0)
#elif BITS == 24
-#define COPY_PIXEL(to, from) \
- *(to++) = from; *(to++) = (from) >> 8; *(to++) = (from) >> 16
+#define COPY_PIXEL(to, from) \
+ do { \
+ *(to++) = from; \
+ *(to++) = (from) >> 8; \
+ *(to++) = (from) >> 16; \
+ } while(0)
#elif BITS == 32
-#define COPY_PIXEL(to, from) *(uint32_t *)to = from; to += 4;
+#define COPY_PIXEL(to, from) do { *(uint32_t *)to = from; to += 4; } while(0)
#else
#error unknown bit depth
#endif
diff --git a/hw/display/pxa2xx_template.h b/hw/display/pxa2xx_template.h
index 1cbe36c..9aed90e 100644
--- a/hw/display/pxa2xx_template.h
+++ b/hw/display/pxa2xx_template.h
@@ -11,14 +11,26 @@
# define SKIP_PIXEL(to) to += deststep
#if BITS == 8
-# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
#elif BITS == 15 || BITS == 16
-# define COPY_PIXEL(to, from) *(uint16_t *) to = from; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) \
+ do { \
+ *(uint16_t *) to = from; \
+ SKIP_PIXEL(to); \
+ } while(0)
#elif BITS == 24
-# define COPY_PIXEL(to, from) \
- *(uint16_t *) to = from; *(to + 2) = (from) >> 16; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) \
+ do { \
+ *(uint16_t *) to = from; \
+ *(to + 2) = (from) >> 16; \
+ SKIP_PIXEL(to); \
+ } while(0)
#elif BITS == 32
-# define COPY_PIXEL(to, from) *(uint32_t *) to = from; SKIP_PIXEL(to)
+# define COPY_PIXEL(to, from) \
+ do { \
+ *(uint32_t *) to = from; \
+ SKIP_PIXEL(to); \
+ } while(0)
#else
# error unknown bit depth
#endif
diff --git a/hw/display/tc6393xb_template.h b/hw/display/tc6393xb_template.h
index 154aafd..964001f 100644
--- a/hw/display/tc6393xb_template.h
+++ b/hw/display/tc6393xb_template.h
@@ -22,14 +22,18 @@
*/
#if BITS == 8
-# define SET_PIXEL(addr, color) *(uint8_t*)addr = color;
+# define SET_PIXEL(addr, color) *(uint8_t*)addr = color
#elif BITS == 15 || BITS == 16
-# define SET_PIXEL(addr, color) *(uint16_t*)addr = color;
+# define SET_PIXEL(addr, color) *(uint16_t*)addr = color
#elif BITS == 24
-# define SET_PIXEL(addr, color) \
- addr[0] = color; addr[1] = (color) >> 8; addr[2] = (color) >> 16;
+# define SET_PIXEL(addr, color) \
+ do { \
+ addr[0] = color; \
+ addr[1] = (color) >> 8; \
+ addr[2] = (color) >> 16; \
+ } while(0)
#elif BITS == 32
-# define SET_PIXEL(addr, color) *(uint32_t*)addr = color;
+# define SET_PIXEL(addr, color) *(uint32_t*)addr = color
#else
# error unknown bit depth
#endif
--
1.8.4.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH target-arm v2] display: avoid multi-statement macro
2014-01-24 19:08 [Qemu-devel] [PATCH target-arm v2] display: avoid multi-statement macro Paolo Bonzini
@ 2014-01-27 14:56 ` Laszlo Ersek
2014-01-27 17:58 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Laszlo Ersek @ 2014-01-27 14:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: peter.maydell, qemu-devel
On 01/24/14 20:08, Paolo Bonzini wrote:
> For blizzard, pl110 and tc6393xb this is harmless, but for pxa2xx
> Coverity noticed that it is used inside an "if" statement.
> Fix it because it's the file with the highest number of defects
> in the whole QEMU tree! Use "do...while(0)", or just remove the
> semicolon if there's a single statement in the macro.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/display/blizzard_template.h | 22 ++++++++++++++++------
> hw/display/pl110_template.h | 12 ++++++++----
> hw/display/pxa2xx_template.h | 22 +++++++++++++++++-----
> hw/display/tc6393xb_template.h | 14 +++++++++-----
> 4 files changed, 56 insertions(+), 27 deletions(-)
>
> diff --git a/hw/display/blizzard_template.h b/hw/display/blizzard_template.h
> index a8a8899..c48a9b3 100644
> --- a/hw/display/blizzard_template.h
> +++ b/hw/display/blizzard_template.h
> @@ -21,21 +21,31 @@
> #define SKIP_PIXEL(to) to += deststep
> #if DEPTH == 8
> # define PIXEL_TYPE uint8_t
> -# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
> # define COPY_PIXEL1(to, from) *to ++ = from
> #elif DEPTH == 15 || DEPTH == 16
> # define PIXEL_TYPE uint16_t
> -# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
> # define COPY_PIXEL1(to, from) *to ++ = from
> #elif DEPTH == 24
> # define PIXEL_TYPE uint8_t
> -# define COPY_PIXEL(to, from) \
> - to[0] = from; to[1] = (from) >> 8; to[2] = (from) >> 16; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) \
> + do { \
> + to[0] = from; \
> + to[1] = (from) >> 8; \
> + to[2] = (from) >> 16; \
> + SKIP_PIXEL(to); \
> + } while(0)
> +
> # define COPY_PIXEL1(to, from) \
> - *to ++ = from; *to ++ = (from) >> 8; *to ++ = (from) >> 16
> + do { \
> + *to ++ = from; \
> + *to ++ = (from) >> 8; \
> + *to ++ = (from) >> 16; \
> + } while(0)
> #elif DEPTH == 32
> # define PIXEL_TYPE uint32_t
> -# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
> # define COPY_PIXEL1(to, from) *to ++ = from
> #else
> # error unknown bit depth
> diff --git a/hw/display/pl110_template.h b/hw/display/pl110_template.h
> index e738e4a..f685b26 100644
> --- a/hw/display/pl110_template.h
> +++ b/hw/display/pl110_template.h
> @@ -14,12 +14,16 @@
> #if BITS == 8
> #define COPY_PIXEL(to, from) *(to++) = from
> #elif BITS == 15 || BITS == 16
> -#define COPY_PIXEL(to, from) *(uint16_t *)to = from; to += 2;
> +#define COPY_PIXEL(to, from) do { *(uint16_t *)to = from; to += 2; } while(0)
> #elif BITS == 24
> -#define COPY_PIXEL(to, from) \
> - *(to++) = from; *(to++) = (from) >> 8; *(to++) = (from) >> 16
> +#define COPY_PIXEL(to, from) \
> + do { \
> + *(to++) = from; \
> + *(to++) = (from) >> 8; \
> + *(to++) = (from) >> 16; \
> + } while(0)
> #elif BITS == 32
> -#define COPY_PIXEL(to, from) *(uint32_t *)to = from; to += 4;
> +#define COPY_PIXEL(to, from) do { *(uint32_t *)to = from; to += 4; } while(0)
> #else
> #error unknown bit depth
> #endif
> diff --git a/hw/display/pxa2xx_template.h b/hw/display/pxa2xx_template.h
> index 1cbe36c..9aed90e 100644
> --- a/hw/display/pxa2xx_template.h
> +++ b/hw/display/pxa2xx_template.h
> @@ -11,14 +11,26 @@
>
> # define SKIP_PIXEL(to) to += deststep
> #if BITS == 8
> -# define COPY_PIXEL(to, from) *to = from; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while(0)
> #elif BITS == 15 || BITS == 16
> -# define COPY_PIXEL(to, from) *(uint16_t *) to = from; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) \
> + do { \
> + *(uint16_t *) to = from; \
> + SKIP_PIXEL(to); \
> + } while(0)
> #elif BITS == 24
> -# define COPY_PIXEL(to, from) \
> - *(uint16_t *) to = from; *(to + 2) = (from) >> 16; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) \
> + do { \
> + *(uint16_t *) to = from; \
> + *(to + 2) = (from) >> 16; \
> + SKIP_PIXEL(to); \
> + } while(0)
> #elif BITS == 32
> -# define COPY_PIXEL(to, from) *(uint32_t *) to = from; SKIP_PIXEL(to)
> +# define COPY_PIXEL(to, from) \
> + do { \
> + *(uint32_t *) to = from; \
> + SKIP_PIXEL(to); \
> + } while(0)
> #else
> # error unknown bit depth
> #endif
> diff --git a/hw/display/tc6393xb_template.h b/hw/display/tc6393xb_template.h
> index 154aafd..964001f 100644
> --- a/hw/display/tc6393xb_template.h
> +++ b/hw/display/tc6393xb_template.h
> @@ -22,14 +22,18 @@
> */
>
> #if BITS == 8
> -# define SET_PIXEL(addr, color) *(uint8_t*)addr = color;
> +# define SET_PIXEL(addr, color) *(uint8_t*)addr = color
> #elif BITS == 15 || BITS == 16
> -# define SET_PIXEL(addr, color) *(uint16_t*)addr = color;
> +# define SET_PIXEL(addr, color) *(uint16_t*)addr = color
> #elif BITS == 24
> -# define SET_PIXEL(addr, color) \
> - addr[0] = color; addr[1] = (color) >> 8; addr[2] = (color) >> 16;
> +# define SET_PIXEL(addr, color) \
> + do { \
> + addr[0] = color; \
> + addr[1] = (color) >> 8; \
> + addr[2] = (color) >> 16; \
> + } while(0)
> #elif BITS == 32
> -# define SET_PIXEL(addr, color) *(uint32_t*)addr = color;
> +# define SET_PIXEL(addr, color) *(uint32_t*)addr = color
> #else
> # error unknown bit depth
> #endif
>
I kinda have to wash out my eyes now (because of the original code of
course; "*to ++", seriously? :))
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH target-arm v2] display: avoid multi-statement macro
2014-01-24 19:08 [Qemu-devel] [PATCH target-arm v2] display: avoid multi-statement macro Paolo Bonzini
2014-01-27 14:56 ` Laszlo Ersek
@ 2014-01-27 17:58 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2014-01-27 17:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On 24 January 2014 19:08, Paolo Bonzini <paolo.bonzini@gmail.com> wrote:
> For blizzard, pl110 and tc6393xb this is harmless, but for pxa2xx
> Coverity noticed that it is used inside an "if" statement.
> Fix it because it's the file with the highest number of defects
> in the whole QEMU tree! Use "do...while(0)", or just remove the
> semicolon if there's a single statement in the macro.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
checkpatch says:
total: 29 errors, 0 warnings, 111 lines checked
some of which are genuinely new with this patch.
In general it's better to fix the nits if you're messing
with the file anyway; could you do that, please?
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-01-27 17:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-24 19:08 [Qemu-devel] [PATCH target-arm v2] display: avoid multi-statement macro Paolo Bonzini
2014-01-27 14:56 ` Laszlo Ersek
2014-01-27 17:58 ` Peter Maydell
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).