Comment 15 for bug 1639372

(In reply to Seth Arnold from comment #7)
> stride = cairo_format_stride_for_width(format, width);

I think that function should return a ptrdiff_t. If it does so the compiler will start issuing warnings on 64-bit builds where the result is truncated to 32 bits, as in the above assignment.

I believe that in general it is ok for 'width' and 'height' to be (int), assuming you don't support 16-bit systems, but any multiplication has to be done with care:

> ptr = malloc (stride* height);

That strikes me as bogus. Images with rows as long as 2^31 bytes are rare; pretty much limited to people trying to break libpng! However images with 2^31 or more bytes in total can happen and it is perfectly possible on a modern desktop/laptop/tablet to end up with one in memory. (Maybe it's a little dumb, but it is possible.)

In any case if cairo_format_stride_for_width uses (-1) as an error return (I'm not that was what you are saying) the callers need to check for it.

> surface = cairo_image_surface_create_for_data (ptr, format,
> width, height, stride);

The last (stride) parameter should have type ptrdiff_t. I think the point I was trying to make before is that if the change is done in the struct and places that use the 'stride' member it will force a lot of other changes (int to ptrdiff_t) but a GNU 64-bit compiler with -Wall -Wextra should show the vast majority of the issues.

> Here's another example:
>
> static cairo_status_t
> _cairo_image_spans_and_zero (void *abstract_renderer,
> int y, int height,
> const cairo_half_open_span_t *spans,
> unsigned num_spans)
> {
> cairo_image_span_renderer_t *r = abstract_renderer;
> uint8_t *mask;
> int len;
>
> mask = r->u.mask.data;
> if (y > r->u.mask.extents.y) {
> len = (y - r->u.mask.extents.y) * r->u.mask.stride;
> memset (mask, 0, len);
> mask += len;
> }
>
>
> The u.mask.stride would be a ptrdiff_t after this patch, but 'len' is still
> only an 'int'.

Right; that should result in a compiler warning for the assignment to 'len', so then the next step is to change 'len' to ptrdiff_t. Fixing the warnings forces the change through to all the required places. The only way to stop the warning without fixing the problem is to use an explicit cast; a static_cast in C++ terms. casts are evil ;-)