Created attachment 37187 [details]
Test program showing non-terminating attribute loop

Hmm...

Pango attributes have unsigned ranges, iterators have signed ranges.
Consequently iterators cannot really walk over attributes wihtout an API
change.  Fun.

In the short run I guess we'll just have to shove unsigned numbers into the
iterators signed slots.  The only visible part will be the output for
pango_attr_iterator_range.

Created attachment 37499 [details] [review]
Suggested patch

This patch makes iterator indices guint thus eliminating some very
questionable MIN(int,guint) operations.

This is necessary, but leaves pango_attr_iterator_range screwed: it's output
args remain of type gint.

The test program now reads...

		guint start, end;
		pango_attr_iterator_range (iter, (gint*)&start, (gint*)&end);
		g_print ("start=%u end=%u\n", start, end);

...demonstrating the incredible new ugly way that pango_attr_iterator_range
should be called.  the program now prints

start=0 end=2147483653
start=2147483653 end=4294967295

as it should.  Note, that those indices would be <0 if they were signed.

Created attachment 37502 [details] [review]
Updated patch

It turns out a lot of places are mixing signed and unsigned.  This isn't
exactly
pretty.

Also, it appears that the advance-iterator-to operation is generally useful
and should be available for internal as well as external use.  I had to fix
three copies of that.

pango_attr_iterator_range() could clamp it's reported values to [0,G_MAXINT],
that's going to be better than reporting end_index < start_index.

Ah, caught between a rock and a soft place.

I guess you could do that.  It violates what pango attributes have been
promising, but only for index values where no-one really cares.

Doing it only moves the problem, though.  You would be left with checking
that no-one really expects to be able to iterate to a certain point and
you still have signed vs unsigned comparisons to deal with.

The signed/unsigned comparisons were easily fixed; changed PangoAttrIterator
to have guint, compiled with -W to get gcc to complain about signed/unsigned
comparisons and cleaned them up.

In terms of callers - we don't introduce any *new* bugs, since there was
no way of returning more than G_MAXINT anyways. Many of the callers 
of pango_iterator_range() could theoretically get into infinite loops
given input text that exceeded G_MAXINT bytes. I put in some quick fixes
to at least avoid the infinite loops, and also made pango_layout_set_text()
more reliably at truncating really long input text. 

There are almost certainly remaining problems here, but 2GB of text is
pretty big compared to the speed and per-character memory usage 
of Pango... it's not a huge concern of mine at the moment, really.

I think my current fixes should handle the cases where someone uses
G_MAXINT or G_MAXUINT to mean "infinity" in an attribute list.

File Edit Options Buffers Tools Help
2005-07-26  Owen Taylor  <otaylor@redhat.com>

        Fixes for signed/unsigned in PangoAttrIterator ((#166700,
        Morten Welinder)

        * pango/pango-attributes.c (pango_attr_iterator_range):
        Clamp results to G_MAXINT to avoid negative numbers from
        signed/unsigned conversions.

        * pango/pango-attributes.c: Make PangoAttrIterator
        start_index/end_index unsigned to match PangoAttribute.
        Change various local variables to match.

        * pango/ellipsize.c (advance_iterator_to)
        pango/pango-attributes.c (pango_attr_iterator_range)
        pango/pango-glyph-item.c (pango_glyph_item_apply_attrs)
        pango/pango-layout.c (pango_layout_line_get_empty_extents):
        Always check the return value from pango_attr_iterator()
        to deal with potential infinite loops when trying to
        advance to position G_MAXINT.

        * pango/pango-layout.c (pango_layout_set_text): Handle
        the case where the text passed in is longer than
        than G_MAXINT and length < 0.