Fix bug in fimage_composite()

This function was creating a new image containing all the intersection
fragments from source and dest, but also adding each of the
destination fragments. Unfortunately, this was totally broken because
the destination fragments would be added many times to the new image
without corresponding ref counts, resulting in double free and other
memory corruption.

The fix instead makes fragment_composite() just return a new fragment
without touching dest, and fimage_composite() then just makes a copy
of the destination fragment and subtracts the intersection.

3 files changed, 35 insertions, 13 deletions

diff --git a/src/fimage.c b/src/fimage.c
index f0fc30e1..32657ccb 100644
--- a/src/fimage.c
+++ b/src/fimage.c
@@ -1,5 +1,6 @@
 #include <stdlib.h>
 #include <pixman.h>
+#include <stdio.h>
 #include "fimage.h"
 
 /* TODO:
@@ -190,11 +191,14 @@ fimage_composite (fimage_t *dest, pixman_op_t op, fimage_t *source)
 	    new_frag = fragment_composite (dfrag, op, sfrag);
 
 	    new_image = fimage_append_fragment (new_image, new_frag);
+
+	    dfrag = fragment_copy (dfrag);
+	    dfrag = fragment_subtract (dfrag, new_frag);
+
 	    new_image = fimage_append_fragment (new_image, dfrag);
 	}
     }
 
-    dest->n_fragments = 0;
     fimage_free (dest);
 
     return new_image;
diff --git a/src/fimage.h b/src/fimage.h
index f1e4b1b8..9fd54a0d 100644
--- a/src/fimage.h
+++ b/src/fimage.h
@@ -25,6 +25,7 @@ fragment_t *fragment_new_image  (int                   width,
 				 pixman_image_t       *image);
 /* Does *not* take ownership of region */
 fragment_t *fragment_new_region (pixman_region32_t    *region);
+fragment_t *fragment_copy       (fragment_t *fragment);
 
 /* Creates a new fragment corresponding to the intersection of
  * source with destination. The content of this fragment is
diff --git a/src/fragment.c b/src/fragment.c
index 0a35e885..d647b40c 100644
--- a/src/fragment.c
+++ b/src/fragment.c
@@ -2,6 +2,7 @@
 #include <stdlib.h>
 #include <pixman.h>
 #include "fimage.h"
+#include <stdio.h>
 
 /* State should be considered a 'value' object that can be copied and written
  * to. It may use copy-on-write internally, but the first iteration will
@@ -87,7 +88,7 @@ state_new (state_type_t type)
     state_t *state = malloc (sizeof *state);
     if (!state)
 	return NULL;
-    
+
     state->common.ref_count = 1;
     state->common.type = type;
     
@@ -139,17 +140,17 @@ state_composite (state_t *dest, pixman_op_t op, state_t *source)
     new_state = malloc (sizeof *new_state);
     if (!new_state)
 	return NULL;
-    
+
     /* FIXME: Always using COMPOSITE should be correct, but
      * we can do a lot better with a few simple optimizations
      */
     new_state->common.type = STATE_COMPOSITE;
     new_state->common.ref_count = 1;
-
+    
     new_state->composite.dest = state_ref (dest);
     new_state->composite.op = op;
     new_state->composite.source = state_ref (source);
-    
+
     return new_state;
 }
 
@@ -159,6 +160,7 @@ static const fragment_t broken_fragment =
     { { 0, 0, 0, 0  }, NULL },	/* region */
     NULL			/* state */
 };
+#include <stdio.h>
 
 static fragment_t *
 fragment_alloc (int width, int height, state_type_t type)
@@ -176,7 +178,7 @@ fragment_alloc (int width, int height, state_type_t type)
     
     fragment->broken = FALSE;
     pixman_region32_init_rect (&fragment->region, 0, 0, width, height);
-    
+
     return fragment;
 }
 
@@ -315,16 +317,31 @@ fragment_intersect (fragment_t *dest, fragment_t *source)
 	return (fragment_t *)&broken_fragment;
     }
     
-    if (!pixman_region32_subtract (
-	    &dest->region, &dest->region, &new_fragment->region))
+    return new_fragment;
+}
+
+fragment_t *
+fragment_copy (fragment_t *fragment)
+{
+    fragment_t *copy;
+
+    if (fragment->broken)
+	return fragment;
+
+    if (!(copy = malloc (sizeof *fragment)))
+	return (fragment_t *)&broken_fragment;
+
+    copy->broken = FALSE;
+
+    pixman_region32_init (&copy->region);
+    if (!pixman_region32_copy (&copy->region, &fragment->region))
     {
-	pixman_region32_fini (&new_fragment->region);
-	free (new_fragment);
-	
+	free (copy);
 	return (fragment_t *)&broken_fragment;
     }
-    
-    return new_fragment;
+
+    copy->state = state_ref (fragment->state);
+    return copy;
 }
 
 /* Compute intersection of dest and source with state corresponding