[cairo] Re: [cairo-commit] src/cairo-ps-surface.c

[Carl Worth]

On Tue, 21 Nov 2006 09:11:27 -0800 (PST),  wrote:
>     [PS] Eliminate compiler warnings about unoptimizable loops
>
>     An innocient-looking loop like this:
>
>       for (j = 0; j <= last; j++)
>         something();
>
>     cannot be optimized, because it may loop forever!
>     Imagine the case that last is MAXINT, the loop will never end.  The correct
>     way to write it is:

Behdad,

Thanks for the fix, and the careful explanation. Just one style nit on
the patch:


> -    if (num_glyphs == 0)
> +    if (num_glyphs <= 0)
>          return CAIRO_STATUS_SUCCESS;
>
> +    n_glyphs = num_glyphs;

That's pretty darn ugly. I had first noticed cases of num_glyphs
getting replaced with n_glyphs and couldn't understand why you were
using a less descriptive identifier, (which would be bad style). But
then I realized we've still got both n_glyphs and num_glyphs in the
same function.

That's fragile, (or if not actually that fragile since we don't
expect to ever change either value, it's still ugly).

How about this instead:

	if (num_glyphs <= 0)
	    return CAIRO_STATUS_SUCCESS;

	num_glyphs_unsigned = num_glyphs;

?

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/cairo/attachments/20061121/928a5f0e/attachment.pgp