it looks like a badly implemented for-loop which gcc is unable to optimize

BTW I got this too with gcc-5.3 and gdk-pixbuf-2.34.0, and its preventing me from installing gdk-pixbuf with the higher security settings I use

Created attachment 343460 [details] [review]
Fix the dithering loop and array access

yep, looks like a valid change to me.
Using DM[0][i] really seems like a quick hack rather than semantically correct.

Review of attachment 343460 [details] [review]:

Today in drive-by nit-picking, it's me!

::: gdk-pixbuf-2.36.3/contrib/gdk-pixbuf-xlib/gdk-pixbuf-xlibrgb.c.orig
@@ +1353,3 @@
     {
       DM_565 = malloc(sizeof(guint32) * DM_WIDTH * DM_HEIGHT);
+      for (i = 0; i < DM_HEIGHT; i++)

Add braces around the loop body of this, since now the body spans multiple lines.

I wonder if, since we already have to add new variables, we might take the opportunity to rename them to r/c or x/y for the DM indices and i for DM_565; and move all of these variables' declarations into the if() body.

/restrains self from changing i++ to ++i

Created attachment 371486 [details] [review]
gdk-pixbuf-xlib: Fix out-of-bounds error in dithering loop

Here is an updated version of the original patch by danny.milo@[...] with the suggestions of Daniel Boles incorporated.

Review of attachment 371486 [details] [review]:

Looks good to me, with a small nitpick.

::: contrib/gdk-pixbuf-xlib/gdk-pixbuf-xlibrgb.c
@@ +1356,2 @@
 	{
+	  for (x = 0; x < DM_WIDTH; x++, i++)

You're not initializing `i` to 0, which means `i++` could do anything — especially in aggressively optimized builds.

I also find it slightly less readable, and we don't use this pattern often in GLib/GTK/GdkPixbuf.

I'd rather have the `i` incremented explicitly inside the inner loop, e.g.:

```
DM_565[i++] = (dith << 20) | dith | (((7 - dith) >> 1) << 10);
```

After initializing `i` either when declaring it, or before entering the outer for loop.

Created attachment 371488 [details] [review]
gdk-pixbuf-xlib: Fix out-of-bounds error in dithering loop

Hi Emmanuele

Thanks for the review.

> Looks good to me, with a small nitpick. [...] You're not initializing `i` to 0.

That's not a small nitpick. That's a big oversight on my side.
I fixed that and explicitly increment i in the loop.

Review of attachment 371488 [details] [review]:

Looks good, thanks for the quick fix!

Created attachment 371489 [details] [review]
gdk-pixbuf-xlib: Fix out-of-bounds error in dithering loop

Sorry, there was an oversight in the patch. The variable i has to be incremented after the print statement.

Review of attachment 371489 [details] [review]:

Nice :thumbsup:

Attachment 371489 [details] pushed as 979a87b - gdk-pixbuf-xlib: Fix out-of-bounds error in dithering loop