Re: [Qemu-devel] [PATCH 1/4] console: a few cleanups

[Stefan Weil <sw@weilnetz.de>]

[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]

Am 18.01.2012 01:49, schrieb Anthony Liguori:
>
> On Jan 17, 2012 12:16 AM, "Stefan Weil" <sw@weilnetz.de 
> <mailto:sw@weilnetz.de>> wrote:
> >
> > Am 16.01.2012 23:36, schrieb Anthony Liguori:
> >
> >> We don't do anything with the list of registered DisplayState so 
> get rid of it.
> >> That's one less list to deal with down the road.
> >>
> >> Also pass DisplayState to the callbacks in DisplayState so users 
> can avoid
> >> global state references.
> >>
> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com 
> <mailto:aliguori@us.ibm.com>>
> >> ---
> >>  console.c          |    9 +++------
> >>  console.h          |    6 ++----
> >>  hw/vmware_vga.c    |    4 ++--
> >>  ui/sdl.c           |    4 ++--
> >>  ui/spice-display.c |    4 ++--
> >>  ui/vnc.c           |    4 ++--
> >>  6 files changed, 13 insertions(+), 18 deletions(-)
> >>
> >
> > Is checkpatch.pl <http://checkpatch.pl> buggy, or did you forget to 
> run it?
> >
> > There are coding style issues at least in patch 1/4 and 2/4.
> Please elaborate.
> Regards,
> Anthony Liguori
> >
> > Regards,
> >
> > Stefan Weil
> >
> >


There are missing braces in several modified blocks, for example:

      if (s->vga.ds->cursor_define)
-        s->vga.ds->cursor_define(qc);
+        s->vga.ds->cursor_define(s->vga.ds, qc);
      cursor_put(qc);

checkpatch.pl does not detect this because the change was in the one 
line block,
not in the conditional statement:

$ scripts/checkpatch.pl 
/home/stefan/Downloads/1-4-console-a-few-cleanups.patch
WARNING: line over 80 characters
#85: FILE: hw/vmware_vga.c:908:
+            s->vga.ds->mouse_set(s->vga.ds, s->cursor.x, s->cursor.y, 
s->cursor.on);

total: 0 errors, 1 warnings, 89 lines checked

It's quite common in other patches to fix the braces when blocks without 
braces
are changed.

Regards,
Stefan Weil


[-- Attachment #2: Type: text/html, Size: 3560 bytes --]