Re: gdk-pixubuf bug + patch

[Mathieu Lacage <mathieu gnu org>]

hi all, 

given the API change involved by this patch, it would be nice if someone
were to give me some feedback on it. (And no, I cannot report a bug in
bugzilla for this because our sysadmin closed port 80 connections
because of this stupid windows worm).

For the record, here is the patch included in the mail this time.

Mathieu

Index: gdk-pixbuf/gdk-pixbuf-scale.c
===================================================================
RCS file: /cvs/gnome/gdk-pixbuf/gdk-pixbuf/gdk-pixbuf-scale.c,v
retrieving revision 1.5
diff -u -r1.5 gdk-pixbuf-scale.c
--- gdk-pixbuf/gdk-pixbuf-scale.c	2000/04/24 05:28:01	1.5
+++ gdk-pixbuf/gdk-pixbuf-scale.c	2001/08/06 16:39:01
@@ -27,6 +27,13 @@
 
 
 
+#define MAXVAL ((1<<15)-1)
+#define MINVAL (((double)1)/(1<<14))
+
+
+#define abs_double(x) ((x)<0)?-(x):(x)
+#define pad(x) ((x)<MINVAL)?MINVAL:((x>MAXVAL)?(MAXVAL):(x))
+
 /**
  * gdk_pixbuf_scale:
  * @src: a #GdkPixbuf
@@ -45,6 +52,11 @@
  * translating by @offset_x and @offset_y, then renders the rectangle
  * (@dest,@dest_y,@dest_width,@dest_height) of the resulting image onto the
  * destination drawable replacing the previous contents.
+ * If @scale_x or @scale_y are negative, their absolute value is used as scaling
+ * factor.
+ * If @scale_x or @scale_y are smaller than (1/1<<14), the (1/1<<14) scaling 
+ * factor is used. If they are bigger than (1<<15-1), the (1<<15-1) scaling factor
+ * is used.
  **/
 void
 gdk_pixbuf_scale (const GdkPixbuf *src,
@@ -66,6 +78,8 @@
 
   offset_x = floor (offset_x + 0.5);
   offset_y = floor (offset_y + 0.5);
+  scale_x  = pad (abs_double (scale_x));
+  scale_y  = pad (abs_double (scale_y));
   
   pixops_scale (dest->pixels + dest_y * dest->rowstride + dest_x * dest->n_channels,
 		dest_x - offset_x, dest_y - offset_y, 
@@ -95,6 +109,11 @@
  * translating by @offset_x and @offset_y, then composites the rectangle
  * (@dest,@dest_y,@dest_width,@dest_height) of the resulting image onto the
  * destination drawable.
+ * If @scale_x or @scale_y are negative, their absolute value is used as scaling
+ * factor.
+ * If @scale_x or @scale_y are smaller than (1/1<<14), the (1/1<<14) scaling 
+ * factor is used. If they are bigger than (1<<15-1), the (1<<15-1) scaling factor
+ * is used.
  **/
 void
 gdk_pixbuf_composite (const GdkPixbuf *src,
@@ -118,6 +137,9 @@
 
   offset_x = floor (offset_x + 0.5);
   offset_y = floor (offset_y + 0.5);
+  scale_x  = pad (abs_double (scale_x));
+  scale_y  = pad (abs_double (scale_y));
+  
   pixops_composite (dest->pixels + dest_y * dest->rowstride + dest_x * dest->n_channels,
 		    dest_x - offset_x, dest_y - offset_y, 
 		    dest_x + dest_width - offset_x, dest_y + dest_height - offset_y,
@@ -152,6 +174,11 @@
  * (@dest,@dest_y,@dest_width,@dest_height) of the resulting image with
  * a checkboard of the colors @color1 and @color2 and renders it onto the
  * destination drawable.
+ * If @scale_x or @scale_y are negative, their absolute value is used as scaling
+ * factor.
+ * If @scale_x or @scale_y are smaller than (1/1<<14), the (1/1<<14) scaling 
+ * factor is used. If they are bigger than (1<<15-1), the (1<<15-1) scaling factor
+ * is used.
  **/
 void
 gdk_pixbuf_composite_color (const GdkPixbuf *src,
@@ -180,6 +207,8 @@
 
   offset_x = floor (offset_x + 0.5);
   offset_y = floor (offset_y + 0.5);
+  scale_x  = pad (abs_double (scale_x));
+  scale_y  = pad (abs_double (scale_y));
   
   pixops_composite_color (dest->pixels + dest_y * dest->rowstride + dest_x * dest->n_channels,
 			  dest_x - offset_x, dest_y - offset_y, 
Index: gdk-pixbuf/pixops/pixops.c
===================================================================
RCS file: /cvs/gnome/gdk-pixbuf/gdk-pixbuf/pixops/pixops.c,v
retrieving revision 1.16
diff -u -r1.16 pixops.c
--- gdk-pixbuf/pixops/pixops.c	2001/03/01 23:23:09	1.16
+++ gdk-pixbuf/pixops/pixops.c	2001/08/06 16:39:04
@@ -47,6 +47,9 @@
   return check_shift;
 }
 
+#define MAXVAL 2147483647 /* 2^31 - 1 */
+#define pad(x) (x > MAXVAL)?(gint32)MAXVAL:((x < -MAXVAL)?(gint32)-MAXVAL:(x))
+
 static void
 pixops_scale_nearest (guchar        *dest_buf,
 		      int            render_x0,
@@ -66,14 +69,21 @@
 		      double         scale_y)
 {
   int i, j;
-  int x;
-  int x_step = (1 << SCALE_SHIFT) / scale_x;
-  int y_step = (1 << SCALE_SHIFT) / scale_y;
+  gint32 x;
+  /* at worst, scale_x = 1/1<<14 which means that x_step = 1<<(14+SCALE_SHIFT) which means NO overflow
+     if SCALE_SHIFT is smaller than 16. */
+  gint32 x_step = (1 << SCALE_SHIFT) / scale_x;
+  gint32 y_step = (1 << SCALE_SHIFT) / scale_y;
+  const guchar *end_src_buf;
 
 #define INNER_LOOP(SRC_CHANNELS,DEST_CHANNELS) 			\
       for (j=0; j < (render_x1 - render_x0); j++)		\
 	{							\
-	  const guchar *p = src + (x >> SCALE_SHIFT) * SRC_CHANNELS;	\
+	  const guchar *p = src + ((x + (1<<(SCALE_SHIFT-1))) >> SCALE_SHIFT) * SRC_CHANNELS;	\
+          if (p > (src+src_rowstride) || p < src)               \
+            {                                                   \
+	      break;                                            \
+	    }                                                   \
 								\
 	  dest[0] = p[0];					\
 	  dest[1] = p[1];					\
@@ -88,15 +98,22 @@
 	    }							\
 								\
 	  dest += DEST_CHANNELS;				\
-	  x += x_step;						\
+	  x += x_step;					        \
 	}
 
+  end_src_buf = src_buf + src_height * src_rowstride;
+
   for (i = 0; i < (render_y1 - render_y0); i++)
     {
-      const guchar *src  = src_buf + (((i + render_y0) * y_step + y_step / 2) >> SCALE_SHIFT) * src_rowstride;
+      const guchar *src  = src_buf + (((i + render_y0) * y_step + (1<<(SCALE_SHIFT-1))/*+ y_step / 2*/) >> SCALE_SHIFT) * src_rowstride;
       guchar       *dest = dest_buf + i * dest_rowstride;
+
+      if (src > end_src_buf || src < src_buf) 
+	{
+	  break;
+	}
 
-      x = render_x0 * x_step + x_step / 2;
+      x = render_x0 * x_step /*+ x_step / 2*/;
 
       if (src_channels == 3)
 	{
@@ -154,22 +171,35 @@
 			  int            overall_alpha)
 {
   int i, j;
-  int x;
-  int x_step = (1 << SCALE_SHIFT) / scale_x;
-  int y_step = (1 << SCALE_SHIFT) / scale_y;
+  gint32 x;
+  gint32 x_step = (1 << SCALE_SHIFT) / scale_x;
+  gint32 y_step = (1 << SCALE_SHIFT) / scale_y;
+  const guchar *end_src_buf;
 
+  end_src_buf = src_buf + src_height*src_rowstride;
+
   for (i = 0; i < (render_y1 - render_y0); i++)
     {
-      const guchar *src  = src_buf + (((i + render_y0) * y_step + y_step / 2) >> SCALE_SHIFT) * src_rowstride;
+      const guchar *src  = src_buf + (((i + render_y0) * y_step + (1<<(SCALE_SHIFT-1))/*y_step / 2*/) >> SCALE_SHIFT) * src_rowstride;
       guchar       *dest = dest_buf + i * dest_rowstride;
+      
+      if (src < src_buf || src > end_src_buf) 
+	{
+	  break;
+	}
 
-      x = render_x0 * x_step + x_step / 2;
+      x = render_x0 * x_step /*+ x_step / 2*/;
       
       for (j=0; j < (render_x1 - render_x0); j++)
 	{
-	  const guchar *p = src + (x >> SCALE_SHIFT) * src_channels;
+	  const guchar *p = src + ((x + (1<<(SCALE_SHIFT-1))) >> SCALE_SHIFT) * src_channels;
           unsigned int  a0;
 
+	  if (p < src || p > (src+src_rowstride)) 
+	    {
+	      break;
+	    }
+
 	  if (src_has_alpha)
 	    a0 = (p[3] * overall_alpha) / 0xff;
 	  else
@@ -237,18 +267,26 @@
 				guint32        color2)
 {
   int i, j;
-  int x;
-  int x_step = (1 << SCALE_SHIFT) / scale_x;
-  int y_step = (1 << SCALE_SHIFT) / scale_y;
+  gint32 x;
+  gint32 x_step = (1 << SCALE_SHIFT) / scale_x;
+  gint32 y_step = (1 << SCALE_SHIFT) / scale_y;
   int r1, g1, b1, r2, g2, b2;
   int check_shift = get_check_shift (check_size);
+  const guchar *end_src_buf;
+
+  end_src_buf = src_buf + src_height*src_rowstride;
 
   for (i = 0; i < (render_y1 - render_y0); i++)
     {
-      const guchar *src  = src_buf + (((i + render_y0) * y_step + y_step/2) >> SCALE_SHIFT) * src_rowstride;
+      const guchar *src  = src_buf + (((i + render_y0) * y_step + (1<<(SCALE_SHIFT-1)) /*y_step/2*/) >> SCALE_SHIFT) * src_rowstride;
       guchar       *dest = dest_buf + i * dest_rowstride;
 
-      x = render_x0 * x_step + x_step / 2;
+      if (src < src_buf || src > end_src_buf)
+	{
+	  break;
+	}
+
+      x = render_x0 * x_step /*+ x_step / 2 */;
       
       if (((i + check_y) >> check_shift) & 1)
 	{
@@ -273,9 +311,14 @@
 
       for (j=0 ; j < (render_x1 - render_x0); j++)
 	{
-	  const guchar *p = src + (x >> SCALE_SHIFT) * src_channels;
+	  const guchar *p = src + ((x + (1<<(SCALE_SHIFT-1))) >> SCALE_SHIFT) * src_channels;
           int a0;
 	  int tmp;
+
+	  if (p < src || p > (src+src_rowstride))
+	    {
+	      break;
+	    }
 
 	  if (src_has_alpha)
 	    a0 = (p[3] * overall_alpha + 0xff) >> 8;

On 06 Aug 2001 21:08:50 +0000, Mathieu Lacage wrote:
> hi all,
> 
> 
> while playing with gdk-pixbuf, I noticed that a call to gdk_pixbuf_scale
> with very small values makes the scaler segfault.
> 
> So, I fixed it in the NEAREST case (patch attached). The other modes
> still segfault: I have tried unsuccessfully to fix them.
> 
> Mathieu
--
Mathieu Lacage <mathieu gnu org>
Portable: <lacage orange fr>