Re: [Qemu-devel] [PATCH v2] cirrus: handle negative pitch in cirrus_invalidate_region()

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v2] cirrus: handle negative pitch in 
cirrus_invalidate_region()
Message-id: 1485352137-29367-1-git-send-email-w.bumil...@proxmox.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/1485352082-16830-1-git-send-email-gr...@kaod.org 
-> patchew/1485352082-16830-1-git-send-email-gr...@kaod.org
 * [new tag] 
patchew/1485352137-29367-1-git-send-email-w.bumil...@proxmox.com -> 
patchew/1485352137-29367-1-git-send-email-w.bumil...@proxmox.com
 - [tag update]  patchew/20170124110151.937-1-berra...@redhat.com -> 
patchew/20170124110151.937-1-berra...@redhat.com
 - [tag update]  patchew/20170124180420.12430-1-pbonz...@redhat.com -> 
patchew/20170124180420.12430-1-pbonz...@redhat.com
Switched to a new branch 'test'
93e5129 cirrus: handle negative pitch in cirrus_invalidate_region()

=== OUTPUT BEGIN ===
Checking PATCH 1/1: cirrus: handle negative pitch in 
cirrus_invalidate_region()...
ERROR: spaces required around that '-' (ctx:VxV)
#31: FILE: hw/display/cirrus_vga.c:665:
+off_begin -= bytesperline-1;
  ^

ERROR: code indent should never use tabs
#37: FILE: hw/display/cirrus_vga.c:671:
+^Iassert(off_cur_end >= off_cur);$

total: 2 errors, 0 warnings, 14 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PATCH v2] cirrus: handle negative pitch in cirrus_invalidate_region()

[Wolfgang Bumiller]

cirrus_invalidate_region() calls memory_region_set_dirty()
on a per-line basis, always ranging from off_begin to
off_begin+bytesperline. With a negative pitch off_begin
marks the top most used address and thus we need to do an
initial shift backwards by a line for negative pitches of
backward blits, otherwise the first iteration covers the
line going from the start offset forwards instead of
backwards.
Additionally since the start address is inclusive, if we
shift by a full `bytesperline` we move to the first address
*not* included in the blit, so we only shift by one less
than bytesperline.

Signed-off-by: Wolfgang Bumiller 
---
Changes to v1:
* Subtract only bytesperline-1 since the original address should be
  included while the byte at `begin-bytesperline` is exclusive.
* Added an assertion to ensure we don't call memory_region_set_dirty()
  with a negative size since it takes unsigned parameters.

 @Gerd: Still unclear: is -1 enough or should this somehow deal with
multiple depths? As far as I can see the backward blit functions
are all only byte based, as opposed to the patternfill functions in
cirrus_vga_rop2.h which have PUTPIXEL() macros for 8/16/24/32 bpp.
CIRRUS_BLTMODE_BACKWARDS only references cirrus_bkwd_rop[16] and
cirrus_bkwd_transp_rop[16][2] which only contain the bkwd rops
from cirrus_vga_rop.h.

 hw/display/cirrus_vga.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index b1a0773..755110f 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -670,9 +670,14 @@ static void cirrus_invalidate_region(CirrusVGAState * s, 
int off_begin,
 int off_cur;
 int off_cur_end;
 
+if (off_pitch < 0) {
+off_begin -= bytesperline-1;
+}
+
 for (y = 0; y < lines; y++) {
off_cur = off_begin;
off_cur_end = (off_cur + bytesperline) & s->cirrus_addr_mask;
+   assert(off_cur_end >= off_cur);
 memory_region_set_dirty(>vga.vram, off_cur, off_cur_end - off_cur);
off_begin += off_pitch;
 }
-- 
2.1.4