<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Thanks Siarhei. I would appreciate it if you could push these two patches for me with the changes you suggested. I had trouble getting git send-email to work with my SMTP server. </div><div class="gmail_quote">Sorry about the confusion with Reviewed-by. I'm more accustomed to working with Bugzilla where things are a little more explicit. </div><div class="gmail_quote"> </div><div class="gmail_quote">-Bill </div><div class="gmail_quote"> On Thu, Feb 4, 2016 at 4:10 PM, Siarhei Siamashka <<a href="mailto:siarhei.siamashka@gmail.com" target="_blank">siarhei.siamashka@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Bill, Thanks for the patches. By the way, they are a little bit mangled, maybe because you did not use "git send-email" to send them? This is easily fixable for small patches like this, but may become a problem for larger patch sets. On Tue, 2 Feb 2016 17:35:28 -0800 Bill McCloskey <<a href="mailto:wmccloskey@mozilla.com">wmccloskey@mozilla.com</a>> wrote: > This test ensures that calling _intersect_rect on an empty > rectangle produces an empty region. > > Reviewed-by: Siarhei Siamashka <<a href="mailto:siarhei.siamashka@gmail.com">siarhei.siamashka@gmail.com</a>> Your "Signed-off-by: Bill McCloskey <<a href="mailto:wmccloskey@mozilla.com">wmccloskey@mozilla.com</a>>" should be also here. Also adding my "Reviewed-by" was a little bit premature because I had no idea how this patch would look in the end. I only promised that getting my "Reviewed-by" would be easy. > --- > test/region-test.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/test/region-test.c b/test/region-test.c > index bfc219b..e5743e8 100644 > --- a/test/region-test.c > +++ b/test/region-test.c > @@ -116,10 +116,19 @@ main () > > assert (pixman_region32_equal (&r1, &r2)); > pixman_region32_fini (&r1); > pixman_region32_fini (&r2); > > } > pixman_image_unref (fill); > > + /* This would produce a region containing an empty > + * rectangle in it. Such regions are considered malformed, > + * but using an empty rectangle for initialization should > + * work. > + */ Do I understand it right that you are trying to describe the implementation details of the current bug in this comment? But after we fix the bug, this description would instantly become obsolete and will just confuse the people reading the code. I think that just using the commit message here would make a perfect comment: "This test ensures that calling _intersect_rect on an empty rectangle produces an empty region." > + pixman_region32_init_rects (&r1, boxes, 3); > + pixman_region32_intersect_rect (&r1, &r1, 11, 11, 0, 0); > + assert (!pixman_region32_not_empty(&r1)); This is not too critical, but having "pixman_region32_fini (&r1);" here would be nice. > + > return 0; > } You can send a v2 patch (trying to use "git send-email" this time). Or if you prefer, I can push this patch with the above mentioned adjustments. -- Best regards, Siarhei Siamashka </blockquote></div> </div></div>