2013/9/13 Søren Sandmann <[email protected]> > LE GARREC Vincent <[email protected]> writes: > > > So, what should I do ? Is it okay to make a cast for every multiplication > > of stride by height ? > > There are a couple of things that have to be kept in mind: > > - The stride can be negative (this corresponds to the lines of the image > being stored upside down), so storing or casting the stride to size_t > probably won't work. >
You're right, I should use ssize_t. > - Even if you fix the multiplication issues, has limitations in other > places that prevents access to large images working well. See > > https://bugs.freedesktop.org/show_bug.cgi?id=46277 > > for example. Fixing the multiplications may still be worthwhile > though, since we may some day get large images working. > I took a look at this bug. To make it works with pixman, struct pixman_vector should uses pixman_fixed_48_16_t instead of pixman_fixed_t (and remove IS_16_16) but it will create a breakage of the API. I think that solution will make that bug disappears but will not make the whole library okay (a bit like my first patch with the stride's problem where I thought that just one line needed to be changed). > > I'm asking because I don't want to work on a patch that will be denied > > because my solution is wrong. > > I don't know what the best way to do it is, but requiring all rowstride > multiplications to have a cast probably isn't going to be maintainable, > because people will forget about it when writing new code. > I spend some time to think about it and I believe that's the simplest solution. If you think that using (ssize_t) will be forget in new code, why not using a macro like #define STRIDE_MULT(stride, y) ((ssize_t)(stride)*(y)) to show that's important to not use just "*" And about forgetting to do the cast in new code, that's what a bug is ^_^ And that's why review is important. Søren >
_______________________________________________ Pixman mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pixman
