Hi Matthias, > The following patch fixes Novell bug 568811: > VNC Installation aborts right in the middle due to an assertion in > Xvnc/libpixman > > The bug seems occur only on *very* special occasions (in this case, only > in SLES, but *not* in SLED, which is based on the same code basis...). > > Backtrace looks as follows: > > #2 0xb71f7baa in __assert_fail () from /lib/libc.so.6 > #3 0xb781feab in pixman_region_append_non_o (y2=<value optimized out>, > y1=<value optimized out>, r_end=<value optimized out>, > r=<value optimized out>, region=<value optimized out>) at > pixman-region.c:670 > #4 pixman_op (y2=<value optimized out>, y1=<value optimized out>, > r_end=<value optimized out>, r=<value optimized out>, > region=<value optimized out>) at pixman-region.c:996 > #5 0xb7820dbe in pixman_region_union (new_reg=0x82ee57c, reg1=0x82ee57c, > reg2=0xbfd77c90) at pixman-region.c:1439 > #6 0x080f023b in miUnion (newReg=0x82ee57c, reg1=0x82ee57c, reg2=0xbfd77c90) > at miregion.c:1005
Thanks for tracking down this issue. Please note that while pixman is not as strict as the X server in who can push to the repository, it is not a complete free-for-all. Committing small, obvious patches that fixes typos or oversights is fine, but don't commit non-obvious stuff without sending it to the mailing list and giving people time to comment on it. When we are this close to stable release, we need to be even more careful about what goes in. Patches committed between now and next week will likely ship without no testing at all. This patch in particular, I don't think shold ship with no testing at all. So please revert it, and we can consider it again for 0.19.x. Comments and questions: - Why are you seeing an assert in the first place? The asserts have been disabled since 0.16.4/0.17.6 because pixman shouldn't take down the X server with asserts for things that would otherwise be harmless. If you are using an older version than those in SLES, then you might want to either upgrade to 0.16.4, or backport ec6de472d042bec05aaa53f9d14bfbe23edbe01e which is the commit that disables asserts. - How does the degenerate region get created? If you call pixman_init_rectangle() with an empty rectangle, the region will be correctly initialized to have the 'data' field point to pixman_region_empty_data_. I believe the various operations all make sure that if the resulting region is empty, they will store a pointer to pixman_region_empty_data_ in the data field. For better or worse, the region code currently relies on this invariant, so users shouldn't be filling out the region data structure themselves. You can argue that this is bad and pixman should cope with regions initialized directly with empty rectangles, and I probably would agree; it may make sense to fix it in pixman 0.20. It certainly does not make sense to make this change with no testing this close to a stable release unless it is demonstrably a serious bug in pixman that is causing crashes. - The patch does not look wrong to me - a region with empty extents should probably be considered empty rather than degenerate. However, unless this issue is truly causing crashes, changing it is the kind of thing that could cause all kinds of mysterious bugs to show up and therefore definitely should not happen between a release candidate and a stable release. Did you carefully review all the code to make sure it always uses PIXMAN_NIL to check for empty regions? - The region code is rather tricky, so when bugs are fixed in it, I'd like to have tests/region-test.c updated to test the issue. That way - we have a clear way to reproduce the bug - we can verify that the bug is really fixed - we make sure it doesn't get reintroduced - Finally, we need much more detail in commit messages than "Fixes Novell bug xxxxxx". Is that even a public bug? I couldn't figure out how to look at it. I did not create a Novell Login though. Soren _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel