On 29.07.20 19:26, Richard Henderson wrote: > On 7/27/20 2:46 PM, Helge Deller wrote: >> - for (i = 0; i < pix_count; i++) { >> + for (i = 0; i < pix_count && offset + i < buf->size; i++) { >> artist_rop8(s, p + offset + pix_count - 1 - i, >> (data & 1) ? (s->plane_mask >> 24) : 0); >> data >>= 1; > > This doesn't look right. > > You're writing to "offset + pix_count - 1 - i" and yet you're checking bounds > vs "offset + i". > > This could be fixed by computing the complete offset into a local variable and > then have an inner if to avoid the write, as you do for the second loop. > > But it would be better to precompute the correct loop bounds.
Thanks for the feedback. Will send out a revised version soon. Helge > > r~ > > >> @@ -398,7 +390,9 @@ static void vram_bit_write(ARTISTState *s, int posx, int >> posy, bool incr_x, >> for (i = 3; i >= 0; i--) { >> if (!(s->image_bitmap_op & 0x20000000) || >> s->vram_bitmask & (1 << (28 + i))) { >> - artist_rop8(s, p + offset + 3 - i, data8[ROP8OFF(i)]); >> + if (offset + 3 - i < buf->size) { >> + artist_rop8(s, p + offset + 3 - i, data8[ROP8OFF(i)]); >> + } >> } >> } >> memory_region_set_dirty(&buf->mr, offset, 3); >> @@ -420,7 +414,7 @@ static void vram_bit_write(ARTISTState *s, int posx, int >> posy, bool incr_x, >> break; >> } >> >> - for (i = 0; i < pix_count; i++) { >> + for (i = 0; i < pix_count && offset + i < buf->size; i++) { >> mask = 1 << (pix_count - 1 - i); >> >> if (!(s->image_bitmap_op & 0x20000000) ||