[cairo] [PATCH] Move all code to do debugging spew into pixman-private.

Soeren Sandmann sandmann at daimi.au.dk
Sat Feb 13 17:33:44 PST 2010


From: Søren Sandmann Pedersen <sandmann at redhat.com>

Rather than the region code having its own little debug system, move
all of it into pixman-private where there is already return_if_fail()
macros etc. These macros are now enabled in development snapshots and
nowhere else. Previously they were never enabled unless you modified
the code.

At the same time, remove all the asserts from the region code since we
can never turn them on anyway, and replace them with
critical_if_fail() macros that will print spew to standard error when
DEBUG is defined.

Finally, also change the debugging spew in pixman-bits-image.c to use
return_val_if_fail() instead of its own fprintf().
---
 pixman/pixman-bits-image.c |   10 +---
 pixman/pixman-private.h    |   59 ++++++++++++++++++++-----
 pixman/pixman-region.c     |  102 +++++++++++++------------------------------
 pixman/pixman-utils.c      |   21 +++++++++
 4 files changed, 102 insertions(+), 90 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 23a3796..e86822b 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -1041,14 +1041,10 @@ pixman_image_create_bits (pixman_format_code_t format,
 
     /* must be a whole number of uint32_t's
      */
-    return_val_if_fail (bits == NULL ||
-                        (rowstride_bytes % sizeof (uint32_t)) == 0, NULL);
+    return_val_if_fail (
+	bits == NULL || (rowstride_bytes % sizeof (uint32_t)) == 0, NULL);
 
-    if (PIXMAN_FORMAT_BPP (format) < PIXMAN_FORMAT_DEPTH (format))
-    {
-	fprintf (stderr, "Bad format passed to pixman_image_create_bits();\n");
-	return NULL;
-    }
+    return_val_if_fail (PIXMAN_FORMAT_BPP (format) < PIXMAN_FORMAT_DEPTH (format), NULL);
 
     if (!bits && width && height)
     {
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index c99b101..96d41e4 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -754,18 +754,37 @@ pixman_region16_copy_from_region32 (pixman_region16_t *dst,
  */
 
 #undef DEBUG
-#define DEBUG 0
 
-#if DEBUG
+/* Turn on debugging depending on what type of release this is
+ */
+#if (((PIXMAN_VERSION_MICRO % 2) == 0) && ((PIXMAN_VERSION_MINOR % 2) == 1))
+
+/* Debugging gets turned on for development releases because these
+ * are the things that end up in bleeding edge distributions such
+ * as Rawhide etc.
+ *
+ * For performance reasons we don't turn it on for stable releases or
+ * random git checkouts. (Random git checkouts are often used for
+ * performance work).
+ */
+
+#    define DEBUG
+
+#endif
+
+#ifdef DEBUG
+
+void
+_pixman_log_error (const char *function, const char *message);
 
 #define return_if_fail(expr)                                            \
     do                                                                  \
     {                                                                   \
-	if (!(expr))                                                    \
-	{                                                               \
-	    fprintf (stderr, "In %s: %s failed\n", FUNC, # expr);	\
-	    return;                                                     \
-	}                                                               \
+	if (!(expr))							\
+	{								\
+	    _pixman_log_error (FUNC, "The expression " # expr " was false"); \
+	    return;							\
+	}								\
     }                                                                   \
     while (0)
 
@@ -773,16 +792,27 @@ pixman_region16_copy_from_region32 (pixman_region16_t *dst,
     do                                                                  \
     {                                                                   \
 	if (!(expr))                                                    \
-	{                                                               \
-	    fprintf (stderr, "In %s: %s failed\n", FUNC, # expr);	\
-	    return (retval);                                            \
-	}                                                               \
+	{								\
+	    _pixman_log_error (FUNC, "The expression " # expr " was false"); \
+	    return (retval);						\
+	}								\
     }                                                                   \
     while (0)
 
+#define critical_if_fail(expr)						\
+    do									\
+    {									\
+	if (!(expr))							\
+	    _pixman_log_error (FUNC, "The expression " # expr " was false"); \
+    }									\
+    while (0)
+
+
 #else
 
-#define return_if_fail(expr)                                            \
+#define _pixman_log_error(f,m) do { } while (0)				\
+
+#define return_if_fail(expr)						\
     do                                                                  \
     {                                                                   \
 	if (!(expr))							\
@@ -798,6 +828,11 @@ pixman_region16_copy_from_region32 (pixman_region16_t *dst,
     }                                                                   \
     while (0)
 
+#define critical_if_fail(expr)						\
+    do									\
+    {									\
+    }									\
+    while (0)
 #endif
 
 /*
diff --git a/pixman/pixman-region.c b/pixman/pixman-region.c
index 9f7515c..8821f09 100644
--- a/pixman/pixman-region.c
+++ b/pixman/pixman-region.c
@@ -66,63 +66,23 @@
 #define GOOD_RECT(rect) ((rect)->x1 < (rect)->x2 && (rect)->y1 < (rect)->y2)
 #define BAD_RECT(rect) ((rect)->x1 > (rect)->x2 || (rect)->y1 > (rect)->y2)
 
-/* Turn on debugging depending on what type of release this is
- */
-#if (((PIXMAN_VERSION_MICRO % 2) == 0) && ((PIXMAN_VERSION_MINOR % 2) == 1))
-/* This is a development snapshot, so we want self-checking in order to
- * catch as many bugs as possible. However, we don't turn on the asserts
- * because that just leads to the X server crashing which leads to
- * people not running the snapshots.
- */
-#    define noFATAL_BUGS
-#    define SELF_CHECKS
-#else
-/* This is either a stable release or a random git checkout. We don't
- * want self checks in either case for performance reasons. (Random
- * git checkouts are often used for performance work
+/* There is a bunch of asserts in the region code that get triggered by
+ * the X server. We shouldn't take down the X server if we can avoid it,
+ * so undefine the assert() here.
+ *
+ * If DEBUG is enabled, various spew will end up in the X server log instead.
  */
-#    define noFATAL_BUGS
-#    define noSELF_CHECKS
-#endif
-
-#ifndef FATAL_BUGS
-#    undef assert
-#    undef abort
-#    define assert(expr)
-#    define abort()
-#endif
-
-#ifdef SELF_CHECKS
-
-static void
-log_region_error (const char *function, const char *message)
-{
-    static int n_messages = 0;
-
-    if (n_messages < 5)
-    {
-	fprintf (stderr,
-		 "*** BUG ***\n"
-		 "%s: %s\n"
-		 "Set a breakpoint on 'log_region_error' to debug\n\n",
-                 function, message);
-
-        abort (); /* This is #defined away unless FATAL_BUGS is defined */
-
-	n_messages++;
-    }
-}
+#ifdef DEBUG
 
 #define GOOD(reg)							\
     do									\
     {									\
 	if (!PREFIX (_selfcheck (reg)))					\
-	    log_region_error (FUNC, "Malformed region " # reg);         \
+	    _pixman_log_error (FUNC, "Malformed region " # reg);	\
     } while (0)
 
 #else
 
-#define log_region_error(function, name)
 #define GOOD(reg)
 
 #endif
@@ -284,7 +244,7 @@ alloc_data (size_t n)
 	}								\
 	ADDRECT (next_rect, nx1, ny1, nx2, ny2);			\
 	region->data->numRects++;					\
-	assert (region->data->numRects <= region->data->size);		\
+	critical_if_fail (region->data->numRects <= region->data->size);		\
     } while (0)
 
 #define DOWNSIZE(reg, numRects)						\
@@ -409,7 +369,7 @@ PREFIX (_init_rect) (region_type_t *	region,
     if (!GOOD_RECT (&region->extents))
     {
         if (BAD_RECT (&region->extents))
-            log_region_error (FUNC, "Invalid rectangle passed");
+            _pixman_log_error (FUNC, "Invalid rectangle passed");
         PREFIX (_init) (region);
         return;
     }
@@ -423,7 +383,7 @@ PREFIX (_init_with_extents) (region_type_t *region, box_type_t *extents)
     if (!GOOD_RECT (extents))
     {
         if (BAD_RECT (extents))
-            log_region_error (FUNC, "Invalid rectangle passed");
+            _pixman_log_error (FUNC, "Invalid rectangle passed");
         PREFIX (_init) (region);
         return;
     }
@@ -601,7 +561,7 @@ pixman_coalesce (region_type_t * region,      /* Region to coalesce		 */
      * Figure out how many rectangles are in the band.
      */
     numRects = cur_start - prev_start;
-    assert (numRects == region->data->numRects - cur_start);
+    critical_if_fail (numRects == region->data->numRects - cur_start);
 
     if (!numRects) return cur_start;
 
@@ -689,8 +649,8 @@ pixman_region_append_non_o (region_type_t * region,
 
     new_rects = r_end - r;
 
-    assert (y1 < y2);
-    assert (new_rects != 0);
+    critical_if_fail (y1 < y2);
+    critical_if_fail (new_rects != 0);
 
     /* Make sure we have enough space for all rectangles to be added */
     RECTALLOC (region, new_rects);
@@ -699,7 +659,7 @@ pixman_region_append_non_o (region_type_t * region,
 
     do
     {
-	assert (r->x1 < r->x2);
+	critical_if_fail (r->x1 < r->x2);
 	ADDRECT (next_rect, r->x1, y1, r->x2, y2);
 	r++;
     }
@@ -824,8 +784,8 @@ pixman_op (region_type_t *  new_reg,               /* Place to store result
     r2 = PIXREGION_RECTS (reg2);
     r2_end = r2 + numRects;
     
-    assert (r1 != r1_end);
-    assert (r2 != r2_end);
+    critical_if_fail (r1 != r1_end);
+    critical_if_fail (r2 != r2_end);
 
     old_data = (region_data_type_t *)NULL;
 
@@ -893,8 +853,8 @@ pixman_op (region_type_t *  new_reg,               /* Place to store result
 	 * rectangle after the last one in the current band for their
 	 * respective regions.
 	 */
-        assert (r1 != r1_end);
-        assert (r2 != r2_end);
+        critical_if_fail (r1 != r1_end);
+        critical_if_fail (r2 != r2_end);
 
         FIND_BAND (r1, r1_band_end, r1_end, r1y1);
         FIND_BAND (r2, r2_band_end, r2_end, r2y1);
@@ -1101,7 +1061,7 @@ pixman_set_extents (region_type_t *region)
     region->extents.x2 = box_end->x2;
     region->extents.y2 = box_end->y2;
 
-    assert (region->extents.y1 < region->extents.y2);
+    critical_if_fail (region->extents.y1 < region->extents.y2);
 
     while (box <= box_end)
     {
@@ -1112,7 +1072,7 @@ pixman_set_extents (region_type_t *region)
         box++;
     }
 
-    assert (region->extents.x1 < region->extents.x2);
+    critical_if_fail (region->extents.x1 < region->extents.x2);
 }
 
 /*======================================================================
@@ -1148,8 +1108,8 @@ pixman_region_intersect_o (region_type_t *region,
 
     next_rect = PIXREGION_TOP (region);
 
-    assert (y1 < y2);
-    assert (r1 != r1_end && r2 != r2_end);
+    critical_if_fail (y1 < y2);
+    critical_if_fail (r1 != r1_end && r2 != r2_end);
 
     do
     {
@@ -1306,8 +1266,8 @@ pixman_region_union_o (region_type_t *region,
     int x1;            /* left and right side of current union */
     int x2;
 
-    assert (y1 < y2);
-    assert (r1 != r1_end && r2 != r2_end);
+    critical_if_fail (y1 < y2);
+    critical_if_fail (r1 != r1_end && r2 != r2_end);
 
     next_rect = PIXREGION_TOP (region);
 
@@ -1377,10 +1337,10 @@ PREFIX (_union_rect) (region_type_t *dest,
     if (!GOOD_RECT (&region.extents))
     {
         if (BAD_RECT (&region.extents))
-            log_region_error (FUNC, "Invalid rectangle passed");
+            _pixman_log_error (FUNC, "Invalid rectangle passed");
 	return PREFIX (_copy) (dest, source);
     }
-    
+
     region.data = NULL;
 
     return PREFIX (_union) (dest, source, &region);
@@ -1870,8 +1830,8 @@ pixman_region_subtract_o (region_type_t * region,
 
     x1 = r1->x1;
 
-    assert (y1 < y2);
-    assert (r1 != r1_end && r2 != r2_end);
+    critical_if_fail (y1 < y2);
+    critical_if_fail (r1 != r1_end && r2 != r2_end);
 
     next_rect = PIXREGION_TOP (region);
 
@@ -1915,7 +1875,7 @@ pixman_region_subtract_o (region_type_t * region,
 	     * Left part of subtrahend covers part of minuend: add uncovered
 	     * part of minuend to region and skip to next subtrahend.
 	     */
-            assert (x1 < r2->x1);
+            critical_if_fail (x1 < r2->x1);
             NEWRECT (region, next_rect, x1, y1, r2->x1, y2);
 
             x1 = r2->x2;
@@ -1957,7 +1917,7 @@ pixman_region_subtract_o (region_type_t * region,
      */
     while (r1 != r1_end)
     {
-        assert (x1 < r1->x2);
+        critical_if_fail (x1 < r1->x2);
 
         NEWRECT (region, next_rect, x1, y1, r1->x2, y2);
 
@@ -2319,7 +2279,7 @@ PREFIX (_reset) (region_type_t *region, box_type_t *box)
 {
     GOOD (region);
 
-    assert (GOOD_RECT (box));
+    critical_if_fail (GOOD_RECT (box));
 
     region->extents = *box;
 
diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index 5441b6b..cd78a3a 100644
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -766,3 +766,24 @@ pixman_region32_copy_from_region16 (pixman_region32_t *dst,
 
     return retval;
 }
+
+#if DEBUG
+
+void
+_pixman_log_error (const char *function, const char *message)
+{
+    static int n_messages = 0;
+
+    if (n_messages < 10)
+    {
+	fprintf (stderr,
+		 "*** BUG ***\n"
+		 "In %s: %s\n"
+		 "Set a breakpoint on '_pixman_log_error' to debug\n\n",
+                 function, message);
+
+	n_messages++;
+    }
+}
+
+#endif
-- 
1.6.0.6



More information about the cairo mailing list