Patchwork qxl/update_area_io: cleanup invalid parameters handling

login
register
mail settings
Submitter Michael Tokarev
Date Sept. 19, 2012, 1:41 p.m.
Message ID <1348062086-23490-1-git-send-email-mjt@msgid.tls.msk.ru>
Download mbox | patch
Permalink /patch/185040/
State New
Headers show

Comments

Michael Tokarev - Sept. 19, 2012, 1:41 p.m.
This cleans up two additions of almost the same code in commits
511b13e2c9 and ccc2960d654.  While at it, make error paths
consistent (always use 'break' instead of 'return').

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Cc: Dunrong Huang <riegamaths@gmail.com>
Cc: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
Gerd Hoffmann - Sept. 20, 2012, 6:41 a.m.
On 09/19/12 15:41, Michael Tokarev wrote:
> This cleans up two additions of almost the same code in commits
> 511b13e2c9 and ccc2960d654.  While at it, make error paths
> consistent (always use 'break' instead of 'return').

Other way around: should be 'return' not 'break'.

cheers,
  Gerd
Michael Tokarev - Sept. 20, 2012, 6:43 a.m.
On 20.09.2012 10:41, Gerd Hoffmann wrote:
> On 09/19/12 15:41, Michael Tokarev wrote:
>> This cleans up two additions of almost the same code in commits
>> 511b13e2c9 and ccc2960d654.  While at it, make error paths
>> consistent (always use 'break' instead of 'return').
> 
> Other way around: should be 'return' not 'break'.

In other, early-added, error-return places it is "break".

For now it is irrelevant actually, since right after the
switch we already have "return", so "break" is now the
same as "return".  For future it might not be the case.

/mjt
Gerd Hoffmann - Sept. 20, 2012, 6:50 a.m.
On 09/20/12 08:43, Michael Tokarev wrote:
> For now it is irrelevant actually, since right after the
> switch we already have "return", so "break" is now the
> same as "return".  For future it might not be the case.

Oh, right.  Patch added to spice patch queue.

thanks,
  Gerd

Patch

diff --git a/hw/qxl.c b/hw/qxl.c
index 33169f3..3c82c2a 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1547,20 +1547,13 @@  async_common:
         if (d->ram->update_surface > d->ssd.num_surfaces) {
             qxl_set_guest_bug(d, "QXL_IO_UPDATE_AREA: invalid surface id %d\n",
                               d->ram->update_surface);
-            return;
+            break;
         }
-        if (update.left >= update.right || update.top >= update.bottom) {
+        if (update.left >= update.right || update.top >= update.bottom ||
+            update.left < 0 || update.top < 0) {
             qxl_set_guest_bug(d,
                     "QXL_IO_UPDATE_AREA: invalid area (%ux%u)x(%ux%u)\n",
                     update.left, update.top, update.right, update.bottom);
-            return;
-        }
-
-        if (update.left < 0 || update.top < 0 || update.left >= update.right ||
-            update.top >= update.bottom) {
-            qxl_set_guest_bug(d, "QXL_IO_UPDATE_AREA: "
-                              "invalid area(%d,%d,%d,%d)\n", update.left,
-                              update.right, update.top, update.bottom);
             break;
         }
         if (async == QXL_ASYNC) {