Re: simple GdkPixbuf fix

[Darin Adler <darin bentspoon com>]

On Tuesday, June 12, 2001, at 12:38  PM, Ron Steinke wrote:

> 	This patch is a simple fix to some ugly code in gdk-pixbuf-data.c.
> Does domeone want to commit this? If not, I'll stick it in bugzilla when
> it comes up again.
>
> Ron Steinke
>
> Index: gtk+/gdk-pixbuf/gdk-pixbuf-data.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk-pixbuf/gdk-pixbuf-data.c,v
> retrieving revision 1.15
> diff -u -r1.15 gdk-pixbuf-data.c
> --- gtk+/gdk-pixbuf/gdk-pixbuf-data.c	2001/02/06 14:46:17	1.15
> +++ gtk+/gdk-pixbuf/gdk-pixbuf-data.c	2001/06/12 19:30:41
> @@ -144,10 +144,7 @@
>
>          length -= 12;
>
> -        /* There's some better way like G_MAXINT/height > rowstride
> -         * but I'm not sure it works, so stick to this for now.
> -         */
> -        if (((double)height) * ((double)rowstride) > (double)G_MAXINT) {
> +        if ((G_MAXINT - 1) / height >= rowstride) {
>                  g_set_error (error,
>                               GDK_PIXBUF_ERROR,
>                               GDK_PIXBUF_ERROR_CORRUPT_IMAGE,

Did you test this change?

It's clear with a casual glance that your code does not do the same thing 
that the old code did. For example, if height is 32 and rowstride is 32, 
your code takes the if statement and sets the error.

I'm pretty sure that

	if ((G_MAXINT - 13) / height < rowstride) {

is the correct check, despite the subtle issue of using a signed-integer. 
The comment does have the < sign backwards, but it's otherwise correct 
except for the constant. The reason we need to subtract 13 is that we need 
to check that the length is not going to overflow, and the length is 
height * rowstride + 13. I can't see any reason to subtract 1.

Also:

	has_alpha = read_bool (&p) != FALSE;

should just be:

	has_alpha = read_bool (&p);

because read_bool is already guaranteed to return only 0 or 1.

    -- Darin