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

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.

        * pango/pango-attributes.c (pango_attr_list_splice): Be
        careful about integer overflow - clamp addition.
        (#163246, Morten Welinder)

4 files changed, 60 insertions, 36 deletions

diff --git a/pango/ellipsize.c b/pango/ellipsize.c
index 39649461..8a7b190a 100644
--- a/pango/ellipsize.c
+++ b/pango/ellipsize.c
@@ -386,14 +386,13 @@ advance_iterator_to (PangoAttrIterator *iter,
 {
   int start, end;
   
-  while (TRUE)
+  do
     {
       pango_attr_iterator_range (iter, &start, &end);
       if (end > new_index)
 	break;
-      
-      pango_attr_iterator_next (iter);
     }
+  while (pango_attr_iterator_next (iter));
 }
 
 /* Updates the shaping of the ellipsis if necessary when we move the
diff --git a/pango/pango-attributes.c b/pango/pango-attributes.c
index ecf41e47..d2fe3ce2 100644
--- a/pango/pango-attributes.c
+++ b/pango/pango-attributes.c
@@ -36,8 +36,8 @@ struct _PangoAttrIterator
 {
   GSList *next_attribute;
   GList *attribute_stack;
-  int start_index;
-  int end_index;
+  guint start_index;
+  guint end_index;
 };
 
 static PangoAttribute *pango_attr_color_new         (const PangoAttrClass *klass,
@@ -1054,7 +1054,7 @@ pango_attr_list_insert_internal (PangoAttrList  *list,
 				 gboolean        before)
 {
   GSList *tmp_list, *prev, *link;
-  gint start_index = attr->start_index;
+  guint start_index = attr->start_index;
 
   if (!list->attributes)
     {
@@ -1161,8 +1161,8 @@ pango_attr_list_change (PangoAttrList  *list,
 			PangoAttribute *attr)
 {
   GSList *tmp_list, *prev, *link;
-  gint start_index = attr->start_index;
-  gint end_index = attr->end_index;
+  guint start_index = attr->start_index;
+  guint end_index = attr->end_index;
 
   g_return_if_fail (list != NULL);
 
@@ -1371,26 +1371,40 @@ pango_attr_list_splice (PangoAttrList *list,
 			gint           len)
 {
   GSList *tmp_list;
+  guint upos, ulen;
   
   g_return_if_fail (list != NULL);
   g_return_if_fail (other != NULL); 
   g_return_if_fail (pos >= 0);
   g_return_if_fail (len >= 0);
- 
+
+  upos = (guint)pos;
+  ulen = (guint)len;
+
+/* This definition only works when a and b are unsigned; overflow
+ * isn't defined in the C standard for signed integers
+ */
+#define CLAMP_ADD(a,b) (((a) + (b) < (a)) ? G_MAXUINT : (a) + (b))
+  
   tmp_list = list->attributes;
   while (tmp_list)
     {
       PangoAttribute *attr = tmp_list->data;
 
-      if (attr->start_index <= pos)
+      if (attr->start_index <= upos)
 	{
-	  if (attr->end_index > pos)
-	    attr->end_index += len;
+	  if (attr->end_index > upos)
+	    attr->end_index = CLAMP_ADD (attr->end_index, ulen);
 	}
       else
 	{
-	  attr->start_index += len;
-	  attr->end_index += len;
+	  /* This could result in a zero length attribute if it
+	   * gets squashed up against G_MAXUINT, but deleting such
+	   * an element could (in theory) suprise the caller, so
+	   * we don't delete it.
+	   */
+	  attr->start_index = CLAMP_ADD (attr->end_index, ulen);
+	  attr->end_index = CLAMP_ADD (attr->end_index, ulen);
 	}
 
       tmp_list = tmp_list->next;
@@ -1400,13 +1414,17 @@ pango_attr_list_splice (PangoAttrList *list,
   while (tmp_list)
     {
       PangoAttribute *attr = pango_attribute_copy (tmp_list->data);
-      attr->start_index += pos;
-      attr->end_index += pos;
+      attr->start_index = CLAMP_ADD (attr->start_index, upos);
+      attr->end_index = CLAMP_ADD (attr->end_index, upos);
 
+      /* Same as above, the attribute could be squashed to zero-length; here
+       * pango_attr_list_change() will take care of deleting it.
+       */
       pango_attr_list_change (list, attr);
       
       tmp_list = tmp_list->next;
     }
+#undef CLAMP_ADD
 }
 
 /**
@@ -1433,7 +1451,7 @@ pango_attr_list_get_iterator (PangoAttrList  *list)
   iterator->end_index = 0;
 
   if (!pango_attr_iterator_next (iterator))
-    iterator->end_index = G_MAXINT;
+    iterator->end_index = G_MAXUINT;
 
   return iterator;
 }
@@ -1444,7 +1462,11 @@ pango_attr_list_get_iterator (PangoAttrList  *list)
  * @start: location to store the start of the range
  * @end: location to store the end of the range
  * 
- * Get the range of the current segment.
+ * Get the range of the current segment. Note that the
+ * stored return values are signed, not unsigned like
+ * the values in #PangoAttribute. To deal with this API
+ * oversight, stored return values that wouldn't fit into
+ * a signed integer are clamped to %G_MAXINT.
  **/
 void
 pango_attr_iterator_range (PangoAttrIterator *iterator,
@@ -1454,9 +1476,9 @@ pango_attr_iterator_range (PangoAttrIterator *iterator,
   g_return_if_fail (iterator != NULL);
 
   if (start)
-    *start = iterator->start_index;
+    *start = MIN (iterator->start_index, G_MAXINT);
   if (end)
-    *end = iterator->end_index;
+    *end = MIN (iterator->end_index, G_MAXINT);
 }
 
 /**
@@ -1478,7 +1500,7 @@ pango_attr_iterator_next (PangoAttrIterator *iterator)
     return FALSE;
 
   iterator->start_index = iterator->end_index;
-  iterator->end_index = G_MAXINT;
+  iterator->end_index = G_MAXUINT;
   
   tmp_list = iterator->attribute_stack;
   while (tmp_list)
diff --git a/pango/pango-glyph-item.c b/pango/pango-glyph-item.c
index 1245ca8c..065871f8 100644
--- a/pango/pango-glyph-item.c
+++ b/pango/pango-glyph-item.c
@@ -483,14 +483,13 @@ pango_glyph_item_apply_attrs (PangoGlyphItem   *glyph_item,
 
   /* Advance the attr iterator to the start of the item
    */
-  while (TRUE)
+  do
     {
       pango_attr_iterator_range (iter, &range_start, &range_end);
       if (range_end > glyph_item->item->offset)
 	break;
-
-      pango_attr_iterator_next (iter);
     }
+  while (pango_attr_iterator_next (iter));
 
   state.segment_attrs = pango_attr_iterator_get_attrs (iter);
   
@@ -505,6 +504,7 @@ pango_glyph_item_apply_attrs (PangoGlyphItem   *glyph_item,
        have_cluster;
        have_cluster = _pango_glyph_item_iter_next_cluster (&state.iter))
     {
+      gboolean have_next;
       
       /* [range_start,range_end] is the first range that intersects
        * the current cluster.
@@ -526,18 +526,17 @@ pango_glyph_item_apply_attrs (PangoGlyphItem   *glyph_item,
        * leaving [range_start,range_end] being the first range that
        * intersects the next cluster.
        */
-      while (TRUE)
+      do
 	{
-	  /* If any ranges end in this cluster, then the next cluster
-	   * goes into a separate segment
-	   */
-	  if (range_end <= state.iter.end_index)
-	    start_new_segment = TRUE;
-
 	  if (range_end > state.iter.end_index) /* Range intersects next cluster */
 	    break;
 
-	  pango_attr_iterator_next (iter);
+	  /* Since ranges end in this cluster, the next cluster goes into a
+	   * separate segment
+	   */
+	  start_new_segment = TRUE;
+
+	  have_next = pango_attr_iterator_next (iter);
 	  pango_attr_iterator_range (iter, &range_start, &range_end);
 
 	  if (range_start >= state.iter.end_index) /* New range doesn't intersect this cluster */
@@ -564,6 +563,7 @@ pango_glyph_item_apply_attrs (PangoGlyphItem   *glyph_item,
 	  state.segment_attrs = g_slist_concat (state.segment_attrs,
 						pango_attr_iterator_get_attrs (iter));
 	}
+      while (have_next);
     }
 
  out:
diff --git a/pango/pango-layout.c b/pango/pango-layout.c
index 96a2f042..933d9f48 100644
--- a/pango/pango-layout.c
+++ b/pango/pango-layout.c
@@ -815,7 +815,10 @@ pango_layout_set_text (PangoLayout *layout,
     {
       if (!g_utf8_validate (text, length, &end))
 	g_warning ("Invalid UTF-8 string passed to pango_layout_set_text()");
-      
+
+      while (end - text > G_MAXINT)
+	end = g_utf8_prev_char (end);
+
       length = end - text;
     }
   
@@ -3773,7 +3776,7 @@ pango_layout_line_get_empty_extents (PangoLayoutLine *line,
 	  PangoAttrIterator *iter = pango_attr_list_get_iterator (layout->attrs);
 	  int start, end;
 	  
-	  while (TRUE)
+	  do
 	    {
 	      pango_attr_iterator_range (iter, &start, &end);
 	      
@@ -3796,9 +3799,9 @@ pango_layout_line_get_empty_extents (PangoLayoutLine *line,
 
 		  break;
 		}
-	      
-	      pango_attr_iterator_next (iter);
+
 	    }
+	  while (pango_attr_iterator_next (iter));
 	  
 	  pango_attr_iterator_destroy (iter);
 	}