[cairo] Catching int overflows in allocations

Behdad Esfahbod behdad at behdad.org
Sat Jun 16 13:55:39 PDT 2007


Thanks Vlad.  Small comments below, mostly about copy/pasted comments.

On Fri, 2007-06-15 at 15:34 -0700, Vladimir Vukicevic wrote:
> +/**
> + * _cairo_malloc3:
> + * @a: first factor
> + * @b: second factor
> + * @ksize: size of each element
> + *
> + * Allocates @a*@b*@ksize memory using malloc(), taking care to not
> + * overflow when doing the multiplication.  Behaves much like
> + * calloc(), except that the returned memory is not set to zero.  The
> + * memory should be freed using free().
> + *
> + * Return value: A pointer to the newly allocated memory, or %NULL in
> + * case of malloc() failure or overflow.
> + */
> +
> +#define _cairo_malloc3(a, b, ksize) \
> +  ((unsigned) (a) >= INT32_MAX / (unsigned) (b) ? NULL : \
> +   (unsigned) ((a)*(b)) >= INT32_MAX / (unsigned) (ksize) ? NULL : \
> +   malloc((unsigned) (a) * (unsigned) (b) * (unsigned) ksize))

Any reason you named the third argument ksize instead of just size?  And
the comments say it's like calloc(), which it's not.  Maybe just say
it's like _cairo_malloc2() with three arguments instead of two.

Do you think we need to document that it's better to use a constant as
the last argument?  It's implied by naming it size, so I'm not sure.

> +/**
> + * _cairo_malloc2k:
> + * @a: first factor
> + * @ksize: size of each element
> + * @z: additional size to allocate
> + *
> + * Allocates @a*@ksize+ at z memory using malloc(), taking care to not
> + * overflow when doing the multiplication.  Behaves much like
> + * calloc(), except that the returned memory is not set to zero.  The
> + * memory should be freed using free().
> + *
> + * Return value: A pointer to the newly allocated memory, or %NULL in
> + * case of malloc() failure or overflow.
> + */ 

Ok, the a, ksize, z naming is definitely not consistent with the
_cairo_malloc2k naming.  n, size, k makes more sense to me.

Also in the patch to CODING_STYLE, you still have the part about
manually doing a*b+k mallocation, while this macro handles that.


Cheers,
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759





More information about the cairo mailing list