After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776945 - tests: Fix possible infinite loops
tests: Fix possible infinite loops
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gdk-pixbuf-maint
gdk-pixbuf-maint
Depends on:
Blocks:
 
 
Reported: 2017-01-06 13:35 UTC by Bastien Nocera
Modified: 2017-01-09 17:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: Fix possible infinite loops (4.86 KB, patch)
2017-01-06 13:35 UTC, Bastien Nocera
none Details | Review
tests: Fix possible infinite loops (4.86 KB, patch)
2017-01-09 17:12 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2017-01-06 13:35:54 UTC
.
Comment 1 Bastien Nocera 2017-01-06 13:35:57 UTC
Created attachment 343021 [details] [review]
tests: Fix possible infinite loops

In the unlikely case that the just created pixbuf is invalid, the
returned sizes would be negative. Ensure that those are positive and
non-zero to avoid possible infinite loops.

Coverity CID 1391987
Coverity CID 1391988
Comment 2 Philip Withnall 2017-01-07 13:08:22 UTC
Review of attachment 343021 [details] [review]:

> Ensure that those are positive and non-zero to avoid possible infinite loops.

Anal nitpick: ‘positive’ includes non-zero. ‘Non-negative’ doesn’t. So you could just say ‘Ensure that those are positive to avoid…’.

But looks good to push in any case. :-)
Comment 3 Philip Withnall 2017-01-07 17:21:54 UTC
Review of attachment 343021 [details] [review]:

::: tests/pixbuf-scale.c
@@ +257,3 @@
   /* Check that the result is all gray (or all white in the case of NEAREST) */
   for (y = 0, row = gdk_pixbuf_get_pixels (scaled);
+       y < scaled_height;

Actually, don’t you want to cast `scaled_height` to `(guint)` here to avoid a warning about a signed/unsigned integer comparison?

Similarly below.
Comment 4 Bastien Nocera 2017-01-09 16:16:49 UTC
(In reply to Philip Withnall from comment #3)
> Review of attachment 343021 [details] [review] [review]:
> 
> ::: tests/pixbuf-scale.c
> @@ +257,3 @@
>    /* Check that the result is all gray (or all white in the case of
> NEAREST) */
>    for (y = 0, row = gdk_pixbuf_get_pixels (scaled);
> +       y < scaled_height;
> 
> Actually, don’t you want to cast `scaled_height` to `(guint)` here to avoid
> a warning about a signed/unsigned integer comparison?
> 
> Similarly below.

I don't get any warnings about it, and I compiled with --enable-debug=yes
Comment 5 Philip Withnall 2017-01-09 16:58:00 UTC
(In reply to Bastien Nocera from comment #4)
> (In reply to Philip Withnall from comment #3)
> > Review of attachment 343021 [details] [review] [review] [review]:
> > 
> > ::: tests/pixbuf-scale.c
> > @@ +257,3 @@
> >    /* Check that the result is all gray (or all white in the case of
> > NEAREST) */
> >    for (y = 0, row = gdk_pixbuf_get_pixels (scaled);
> > +       y < scaled_height;
> > 
> > Actually, don’t you want to cast `scaled_height` to `(guint)` here to avoid
> > a warning about a signed/unsigned integer comparison?
> > 
> > Similarly below.
> 
> I don't get any warnings about it, and I compiled with --enable-debug=yes

OK, looks like I don’t understand the C spec ottomh as well as I thought. Go for it, with or without the cast as you prefer. (I think it would make the code a bit clearer in any case, but I don’t really care.)
Comment 6 Bastien Nocera 2017-01-09 17:12:26 UTC
Created attachment 343173 [details] [review]
tests: Fix possible infinite loops

In the unlikely case that the just created pixbuf is invalid, the
returned sizes would be negative. Ensure that those are positive
to avoid possible infinite loops.

Coverity CID 1391987
Coverity CID 1391988
Comment 7 Bastien Nocera 2017-01-09 17:12:50 UTC
Attachment 343173 [details] pushed as 0144147 - tests: Fix possible infinite loops