On Wednesday 12 January 2011 11:34:03 Soeren Sandmann wrote: > Siarhei Siamashka <[email protected]> writes: > > @@ -2590,6 +2589,8 @@ PREFIX (_init_from_image) (region_type_t *region, > > > > PREFIX(_init) (region); > > > > + assert (region->data); > > + > > > > return_if_fail (image->type == BITS); > > return_if_fail (image->bits.format == PIXMAN_a1); > > I'm a little uncomfortable with adding asserts because if they > trigger, they will take down the X server. (Even though this one can't > possibly trigger). > > Instead, I'd rather use the critical_if_fail() macro. If pixman is > compiled with DEBUG defined, it will print stuff to stderr if the > given expression fails.
Fair enough. An updated patch is attached. Does the patch itself look OK? Based on the logic of this code, my understanding is that the 'region->data' has non NULL value all the time, until the very end of 'region_init_from_image' function, where it may be freed as part of an extra optimization if it happens to be redundant. I want to be sure that I'm not missing anything while trying to make static analyzers happy. -- Best regards, Siarhei Siamashka
From 05f9990022ab71694aa692fd643a29dcb9eee4b0 Mon Sep 17 00:00:00 2001 From: Siarhei Siamashka <[email protected]> Date: Mon, 10 Jan 2011 21:01:16 +0200 Subject: [PATCHv2 3/3] The code in 'bitmap_addrect' already assumes non-null 'reg->data' So the check of 'reg->data' pointer can be safely removed. --- pixman/pixman-region.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pixman/pixman-region.c b/pixman/pixman-region.c index 4e7c8db..142493b 100644 --- a/pixman/pixman-region.c +++ b/pixman/pixman-region.c @@ -2545,8 +2545,7 @@ bitmap_addrect (region_type_t *reg, ((r-1)->y1 == ry1) && ((r-1)->y2 == ry2) && ((r-1)->x1 <= rx1) && ((r-1)->x2 >= rx2)))) { - if (!reg->data || - reg->data->numRects == reg->data->size) + if (reg->data->numRects == reg->data->size) { if (!pixman_rect_alloc (reg, 1)) return NULL; @@ -2590,6 +2589,8 @@ PREFIX (_init_from_image) (region_type_t *region, PREFIX(_init) (region); + critical_if_fail (region->data); + return_if_fail (image->type == BITS); return_if_fail (image->bits.format == PIXMAN_a1); -- 1.7.2.2
_______________________________________________ Pixman mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pixman
