From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59725) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnOPp-0007kE-UH for qemu-devel@nongnu.org; Wed, 18 Jan 2012 00:50:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RnOPo-0005X5-Pw for qemu-devel@nongnu.org; Wed, 18 Jan 2012 00:50:33 -0500 Received: from v220110690675601.yourvserver.net ([78.47.199.172]:44650) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnOPo-0005TV-Hs for qemu-devel@nongnu.org; Wed, 18 Jan 2012 00:50:32 -0500 Message-ID: <4F165DA4.2010001@weilnetz.de> Date: Wed, 18 Jan 2012 06:50:28 +0100 From: Stefan Weil MIME-Version: 1.0 References: <1326753397-18476-1-git-send-email-aliguori@us.ibm.com> <4F151242.5090001@weilnetz.de> In-Reply-To: Content-Type: multipart/alternative; boundary="------------040708020800060905080809" Subject: Re: [Qemu-devel] [PATCH 1/4] console: a few cleanups List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: QEMU Developers This is a multi-part message in MIME format. --------------040708020800060905080809 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Am 18.01.2012 01:49, schrieb Anthony Liguori: > > On Jan 17, 2012 12:16 AM, "Stefan Weil" > 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 > > >> --- > >> 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 > > > > 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 --------------040708020800060905080809 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Am 18.01.2012 01:49, schrieb Anthony Liguori:

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 > >

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
--------------040708020800060905080809--