coccinelle script to enforce qemu/osdep.h macros usage

[Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

Posted by Philippe Mathieu-Daudé 8 years, 6 months ago

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/lm32/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/lm32/translate.c b/target/lm32/translate.c
index 692882f447..a9b159a6e3 100644
--- a/target/lm32/translate.c
+++ b/target/lm32/translate.c
@@ -1185,7 +1185,7 @@ void lm32_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 
     for (i = 0; i < 32; i++) {
         cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
-        if ((i + 1) % 4 == 0) {
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             cpu_fprintf(f, "\n");
         }
     }
-- 
2.13.2

Re: [Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

Posted by Michael Walle 8 years, 6 months ago

Am 2017-07-18 08:09, schrieb Philippe Mathieu-Daudé:
> Applied using the Coccinelle semantic patch 
> scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

QEMU_IS_ALIGNED() sounds like it is used to check if a memory access is 
aligned. Although it does the same, the line in question is used for 
formatted output. I'm not sure if this macro should be used here.

-michael

> ---
>  target/lm32/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/lm32/translate.c b/target/lm32/translate.c
> index 692882f447..a9b159a6e3 100644
> --- a/target/lm32/translate.c
> +++ b/target/lm32/translate.c
> @@ -1185,7 +1185,7 @@ void lm32_cpu_dump_state(CPUState *cs, FILE *f,
> fprintf_function cpu_fprintf,
> 
>      for (i = 0; i < 32; i++) {
>          cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
> -        if ((i + 1) % 4 == 0) {
> +        if (QEMU_IS_ALIGNED(i + 1, 4)) {
>              cpu_fprintf(f, "\n");
>          }
>      }

Re: [Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

Posted by Thomas Huth 8 years, 6 months ago

On 18.07.2017 13:42, Michael Walle wrote:
> Am 2017-07-18 08:09, schrieb Philippe Mathieu-Daudé:
>> Applied using the Coccinelle semantic patch
>> scripts/coccinelle/use_osdep.cocci
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> QEMU_IS_ALIGNED() sounds like it is used to check if a memory access is
> aligned. Although it does the same, the line in question is used for
> formatted output. I'm not sure if this macro should be used here.

+1

I think we should not replace every usage of % blindly. It does really
look wrong in this case here.

 Thomas

Re: [Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

Posted by Philippe Mathieu-Daudé 8 years, 6 months ago

On 07/18/2017 11:37 AM, Thomas Huth wrote:
> On 18.07.2017 13:42, Michael Walle wrote:
>> Am 2017-07-18 08:09, schrieb Philippe Mathieu-Daudé:
>>> Applied using the Coccinelle semantic patch
>>> scripts/coccinelle/use_osdep.cocci
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> QEMU_IS_ALIGNED() sounds like it is used to check if a memory access is
>> aligned. Although it does the same, the line in question is used for
>> formatted output. I'm not sure if this macro should be used here.
> 
> +1
> 
> I think we should not replace every usage of % blindly. It does really
> look wrong in this case here.

Dropped, will wear my glasses next time ;)