Nemanja Lukic <nemanja.lu...@rt-rk.com> writes: > This patch set should address all previous code review remarks.
From looking at this patch set fairly briefly, - There is some confusion about the type of the new memcpy function. Does it return bool or void*? And what precisely does the return value mean? It seems to be used both as "this call succeeded" (boolean) and "copy of the destination pointer" (void *). In my opinion, the memcpy routines should take a pixman_implementation_t * as the first argument, and have no return value. This is similar to the other routines exported by implementations. - I'm not sure the exported pixman_memcpy() and the s/memcpy/pixman_memcpy/g sed job make sense. The new memcpy() infrastructure should primarily be considered a way to support the implementations that wish to provide a faster memcpy() for the various graphics operations, and the various ancillary uses of memcpy() don't really need to super optimized. Also, as mentioned in the last review: - The blt() function in pixman-mips32r2.c is not MIPS specific. It can live in pixman-general and rely on the normal fallback mechanism. And instead of calling the new pixman_memcpy() it could just call implementation->toplevel->memcpy(), which is the usual way to call a lower-level function in pixman. - The fill_buff pointers should be static and defined inside the C files that use them. It's not generally a good idea to define variables in a header file. Søren _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman