Patchwork disas/i386.c: Add explicit braces round empty for-loop body

login
register
mail settings
Submitter Peter Maydell
Date Feb. 2, 2013, 5:17 p.m.
Message ID <1359825474-26661-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/217696/
State New
Headers show

Comments

Peter Maydell - Feb. 2, 2013, 5:17 p.m.
Add explicit braces round an empty for-loop body; this fits
QEMU style and is easier to read than an inconspicuous semicolon
at the end of the line. It also silences a clang warning:

disas/i386.c:4723:49: warning: for loop has empty body [-Wempty-body]
          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
                                                       ^
disas/i386.c:4723:49: note: put the semicolon on a separate line to silence this warning [-Wempty-body]

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is the only clang warning on MacOSX apart from the ones about
deprecated features in audio/coreaudio.c and ui/cocoa.m, which is why
I think it's worth zapping despite it being a style issue in one of the
bits of disassembler code we got from binutils and which aren't generally
in QEMU style.

checkpatch complains about the non-multiple-of-4 indent:
WARNING: suspect code indent for conditional statements (10, 10)
#27: FILE: disas/i386.c:4723:
+          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
+          }

but I didn't see any value in reindenting the whole function just to
shut it up.

At some point in the 1.5 cycle maybe we could update the code to avoid
the deprecated MacOS functions and then we could enable warnings-are-errors
on MacOS too.

 disas/i386.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Andreas Färber - Feb. 2, 2013, 6:34 p.m.
Am 02.02.2013 18:17, schrieb Peter Maydell:
> Add explicit braces round an empty for-loop body; this fits
> QEMU style and is easier to read than an inconspicuous semicolon
> at the end of the line. It also silences a clang warning:
> 
> disas/i386.c:4723:49: warning: for loop has empty body [-Wempty-body]
>           for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
>                                                        ^
> disas/i386.c:4723:49: note: put the semicolon on a separate line to silence this warning [-Wempty-body]
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is the only clang warning on MacOSX apart from the ones about
> deprecated features in audio/coreaudio.c and ui/cocoa.m, which is why
> I think it's worth zapping despite it being a style issue in one of the
> bits of disassembler code we got from binutils and which aren't generally
> in QEMU style.
> 
> checkpatch complains about the non-multiple-of-4 indent:
> WARNING: suspect code indent for conditional statements (10, 10)
> #27: FILE: disas/i386.c:4723:
> +          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
> +          }
> 
> but I didn't see any value in reindenting the whole function just to
> shut it up.
> 
> At some point in the 1.5 cycle maybe we could update the code to avoid
> the deprecated MacOS functions and then we could enable warnings-are-errors
> on MacOS too.

On v10.5.8 I'm seeing some redundant declaration warnings from
disas/s390.c and assumed we wouldn't want to fix/workaround the disas
stuff. Same for some exynos4 file.

Also some uninitialized buf2 in the Realtek NIC or so.

And test-thread-pool breaks due to undefined ___sync_fetch_and_add and
___sync_val_compare_and_swap.

With my setup long broken due to pixman, we never made any progress
fixing the main() mess - Stefan W. replaced asprintf in the command line
code but I'd rather get rid of that in some way similar to here:
http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg03983.html

Cheers,
Andreas
Peter Maydell - Feb. 2, 2013, 8:46 p.m.
On 2 February 2013 18:34, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 02.02.2013 18:17, schrieb Peter Maydell:
>> This is the only clang warning on MacOSX apart from the ones about
>> deprecated features in audio/coreaudio.c and ui/cocoa.m

> On v10.5.8 I'm seeing some redundant declaration warnings from
> disas/s390.c and assumed we wouldn't want to fix/workaround the disas
> stuff. Same for some exynos4 file.
>
> Also some uninitialized buf2 in the Realtek NIC or so.

I don't see any of these with 10.6.8 (with either clang or
the Apple gcc). Do they appear to be legitimate warnings?

> And test-thread-pool breaks due to undefined ___sync_fetch_and_add and
> ___sync_val_compare_and_swap.

Hmm, I hadn't tried running make check. I see:
(a) tests randomly hang (possibly only if built with gcc? hmm)
(b) test-string-input-visitor(59002) malloc: *** error for object
0x12068: pointer being freed was not allocated
(c) test-thread-pool seems to hang
(d) make distclean doesn't delete the test executables in test/ :-)
(e) lots of format-security warnings from check-qjson.c:
tests/check-qjson.c:53:33: warning: format string is not a string
literal (potentially insecure) [-Wformat-security]
        obj = qobject_from_json(test_cases[i].encoded);
                                ^~~~~~~~~~~~~~~~~~~~~

I'll prod a bit further on (b) at least...

-- PMM
Peter Maydell - Feb. 18, 2013, 10:29 a.m.
Post-unfreeze ping for trivial...
( patchwork url: http://patchwork.ozlabs.org/patch/217696/ )

thanks
-- PMM

On 2 February 2013 17:17, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add explicit braces round an empty for-loop body; this fits
> QEMU style and is easier to read than an inconspicuous semicolon
> at the end of the line. It also silences a clang warning:
>
> disas/i386.c:4723:49: warning: for loop has empty body [-Wempty-body]
>           for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
>                                                        ^
> disas/i386.c:4723:49: note: put the semicolon on a separate line to silence this warning [-Wempty-body]
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is the only clang warning on MacOSX apart from the ones about
> deprecated features in audio/coreaudio.c and ui/cocoa.m, which is why
> I think it's worth zapping despite it being a style issue in one of the
> bits of disassembler code we got from binutils and which aren't generally
> in QEMU style.
>
> checkpatch complains about the non-multiple-of-4 indent:
> WARNING: suspect code indent for conditional statements (10, 10)
> #27: FILE: disas/i386.c:4723:
> +          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
> +          }
>
> but I didn't see any value in reindenting the whole function just to
> shut it up.
>
> At some point in the 1.5 cycle maybe we could update the code to avoid
> the deprecated MacOS functions and then we could enable warnings-are-errors
> on MacOS too.
>
>  disas/i386.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/disas/i386.c b/disas/i386.c
> index 3b006b1..dbecf1f 100644
> --- a/disas/i386.c
> +++ b/disas/i386.c
> @@ -4720,7 +4720,8 @@ print_operand_value (char *buf, size_t bufsize, int hex, bfd_vma disp)
>           buf[0] = '0';
>           buf[1] = 'x';
>            snprintf_vma (tmp, sizeof(tmp), disp);
> -         for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
> +          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
> +          }
>            pstrcpy (buf + 2, bufsize - 2, tmp + i);
>         }
>        else
> --
> 1.7.11.4
>
>
Blue Swirl - Feb. 23, 2013, 4:49 p.m.
Thanks, applied.

On Sat, Feb 2, 2013 at 5:17 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add explicit braces round an empty for-loop body; this fits
> QEMU style and is easier to read than an inconspicuous semicolon
> at the end of the line. It also silences a clang warning:
>
> disas/i386.c:4723:49: warning: for loop has empty body [-Wempty-body]
>           for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
>                                                        ^
> disas/i386.c:4723:49: note: put the semicolon on a separate line to silence this warning [-Wempty-body]
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This is the only clang warning on MacOSX apart from the ones about
> deprecated features in audio/coreaudio.c and ui/cocoa.m, which is why
> I think it's worth zapping despite it being a style issue in one of the
> bits of disassembler code we got from binutils and which aren't generally
> in QEMU style.
>
> checkpatch complains about the non-multiple-of-4 indent:
> WARNING: suspect code indent for conditional statements (10, 10)
> #27: FILE: disas/i386.c:4723:
> +          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
> +          }
>
> but I didn't see any value in reindenting the whole function just to
> shut it up.
>
> At some point in the 1.5 cycle maybe we could update the code to avoid
> the deprecated MacOS functions and then we could enable warnings-are-errors
> on MacOS too.
>
>  disas/i386.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/disas/i386.c b/disas/i386.c
> index 3b006b1..dbecf1f 100644
> --- a/disas/i386.c
> +++ b/disas/i386.c
> @@ -4720,7 +4720,8 @@ print_operand_value (char *buf, size_t bufsize, int hex, bfd_vma disp)
>           buf[0] = '0';
>           buf[1] = 'x';
>            snprintf_vma (tmp, sizeof(tmp), disp);
> -         for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
> +          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
> +          }
>            pstrcpy (buf + 2, bufsize - 2, tmp + i);
>         }
>        else
> --
> 1.7.11.4
>
>

Patch

diff --git a/disas/i386.c b/disas/i386.c
index 3b006b1..dbecf1f 100644
--- a/disas/i386.c
+++ b/disas/i386.c
@@ -4720,7 +4720,8 @@  print_operand_value (char *buf, size_t bufsize, int hex, bfd_vma disp)
 	  buf[0] = '0';
 	  buf[1] = 'x';
           snprintf_vma (tmp, sizeof(tmp), disp);
-	  for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
+          for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++) {
+          }
           pstrcpy (buf + 2, bufsize - 2, tmp + i);
 	}
       else