| Message ID | 1326753397-18476-1-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
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> > --- > 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 buggy, or did you forget to run it? There are coding style issues at least in patch 1/4 and 2/4. Regards, Stefan Weil
On Jan 17, 2012 12:16 AM, "Stefan Weil" <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> >> --- >> 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 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 > >
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
diff --git a/console.c b/console.c index 135394f..1085b07 100644 --- a/console.c +++ b/console.c @@ -1363,12 +1363,9 @@ static void dumb_display_init(void) void register_displaystate(DisplayState *ds) { - DisplayState **s; - s = &display_state; - while (*s != NULL) - s = &(*s)->next; - ds->next = NULL; - *s = ds; + if (display_state == NULL) { + display_state = ds; + } } DisplayState *get_displaystate(void) diff --git a/console.h b/console.h index 9466886..e78b359 100644 --- a/console.h +++ b/console.h @@ -179,10 +179,8 @@ struct DisplayState { struct DisplayAllocator* allocator; struct DisplayChangeListener* listeners; - void (*mouse_set)(int x, int y, int on); - void (*cursor_define)(QEMUCursor *cursor); - - struct DisplayState *next; + void (*mouse_set)(DisplayState *ds, int x, int y, int on); + void (*cursor_define)(DisplayState *ds, QEMUCursor *cursor); }; void register_displaystate(DisplayState *ds); diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index b1885c3..c379d72 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -480,7 +480,7 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s, } if (s->vga.ds->cursor_define) - s->vga.ds->cursor_define(qc); + s->vga.ds->cursor_define(s->vga.ds, qc); cursor_put(qc); } #endif @@ -905,7 +905,7 @@ static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value) s->cursor.on &= (value != SVGA_CURSOR_ON_HIDE); #ifdef HW_MOUSE_ACCEL if (s->vga.ds->mouse_set && value <= SVGA_CURSOR_ON_SHOW) - s->vga.ds->mouse_set(s->cursor.x, s->cursor.y, s->cursor.on); + s->vga.ds->mouse_set(s->vga.ds, s->cursor.x, s->cursor.y, s->cursor.on); #endif break; diff --git a/ui/sdl.c b/ui/sdl.c index 8cafc44..0040ad2 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -905,7 +905,7 @@ static void sdl_fill(DisplayState *ds, int x, int y, int w, int h, uint32_t c) SDL_FillRect(real_screen, &dst, c); } -static void sdl_mouse_warp(int x, int y, int on) +static void sdl_mouse_warp(DisplayState *ds, int x, int y, int on) { if (on) { if (!guest_cursor) @@ -921,7 +921,7 @@ static void sdl_mouse_warp(int x, int y, int on) guest_x = x, guest_y = y; } -static void sdl_mouse_define(QEMUCursor *c) +static void sdl_mouse_define(DisplayState *ds, QEMUCursor *c) { uint8_t *image, *mask; int bpl; diff --git a/ui/spice-display.c b/ui/spice-display.c index 6c302a3..3d1d5b0 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -328,12 +328,12 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) ssd->notify++; } if (ssd->cursor) { - ssd->ds->cursor_define(ssd->cursor); + ssd->ds->cursor_define(ssd->ds, ssd->cursor); cursor_put(ssd->cursor); ssd->cursor = NULL; } if (ssd->mouse_x != -1 && ssd->mouse_y != -1) { - ssd->ds->mouse_set(ssd->mouse_x, ssd->mouse_y, 1); + ssd->ds->mouse_set(ssd->ds->ssd->mouse_x, ssd->mouse_y, 1); ssd->mouse_x = -1; ssd->mouse_y = -1; } diff --git a/ui/vnc.c b/ui/vnc.c index 1869a7a..3f3d8c3 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -779,7 +779,7 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int } } -static void vnc_mouse_set(int x, int y, int visible) +static void vnc_mouse_set(DisplayState *ds, int x, int y, int visible) { /* can we ask the client(s) to move the pointer ??? */ } @@ -806,7 +806,7 @@ static int vnc_cursor_define(VncState *vs) return -1; } -static void vnc_dpy_cursor_define(QEMUCursor *c) +static void vnc_dpy_cursor_define(DisplayState *ds, QEMUCursor *c) { VncDisplay *vd = vnc_display; VncState *vs;
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> --- 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(-)