Address offline reviews

src/hb-buffer.cc

@@ -552,10 +552,10 @@ hb_buffer_t::merge_clusters_impl (unsigned int start,
   if (cluster_level == HB_BUFFER_CLUSTER_LEVEL_CHARACTERS)
     return;
 
-  unsigned int cluster = info[start].cluster;
+  uint32_t cluster = info[start].cluster;
 
   for (unsigned int i = start + 1; i < end; i++)
-    cluster = MIN<unsigned int> (cluster, info[i].cluster);
+    cluster = MIN (cluster, info[i].cluster);
 
   /* Extend end */
   while (end < len && info[end - 1].cluster == info[end].cluster)
@@ -583,10 +583,10 @@ hb_buffer_t::merge_out_clusters (unsigned int start,
   if (unlikely (end - start < 2))
     return;
 
-  unsigned int cluster = out_info[start].cluster;
+  uint32_t cluster = out_info[start].cluster;
 
   for (unsigned int i = start + 1; i < end; i++)
-    cluster = MIN<unsigned int> (cluster, out_info[i].cluster);
+    cluster = MIN (cluster, out_info[i].cluster);
 
   /* Extend start */
   while (start && out_info[start - 1].cluster == out_info[start].cluster)

src/hb-common.cc

@@ -670,6 +670,30 @@ parse_uint (const char **pp, const char *end, unsigned int *pv)
   return true;
 }
 
+static bool
+parse_uint32 (const char **pp, const char *end, uint32_t *pv)
+{
+  char buf[32];
+  unsigned int len = MIN (ARRAY_LENGTH (buf) - 1, (unsigned int) (end - *pp));
+  strncpy (buf, *pp, len);
+  buf[len] = '\0';
+
+  char *p = buf;
+  char *pend = p;
+  unsigned int v;
+
+  /* Intentionally use strtol instead of strtoul, such that
+   * -1 turns into "big number"... */
+  errno = 0;
+  v = strtol (p, &pend, 0);
+  if (errno || p == pend)
+    return false;
+
+  *pv = v;
+  *pp += pend - p;
+  return true;
+}
+
 static bool
 parse_float (const char **pp, const char *end, float *pv)
 {
@@ -693,7 +717,7 @@ parse_float (const char **pp, const char *end, float *pv)
 }
 
 static bool
-parse_bool (const char **pp, const char *end, unsigned int *pv)
+parse_bool (const char **pp, const char *end, uint32_t *pv)
 {
   parse_space (pp, end);
 
@@ -792,8 +816,8 @@ static bool
 parse_feature_value_postfix (const char **pp, const char *end, hb_feature_t *feature)
 {
   bool had_equal = parse_char (pp, end, '=');
-  bool had_value = parse_uint (pp, end, (unsigned int*) &feature->value) ||
-                   parse_bool (pp, end, (unsigned int*) &feature->value);
+  bool had_value = parse_uint32 (pp, end, &feature->value) ||
+                   parse_bool (pp, end, &feature->value);
   /* CSS doesn't use equal-sign between tag and value.
    * If there was an equal-sign, then there *must* be a value.
    * A value without an eqaul-sign is ok, but not required. */

src/hb-ot-font.cc

@@ -44,7 +44,7 @@
 struct hb_ot_face_metrics_accelerator_t
 {
   unsigned int num_metrics;
-  unsigned int num_advances;
+  uint32_t num_advances;
   unsigned int default_advance;
   unsigned short ascender;
   unsigned short descender;
@@ -138,7 +138,7 @@ struct hb_ot_face_metrics_accelerator_t
 	return this->default_advance;
     }
 
-    return this->table->longMetric[MIN<hb_codepoint_t> (glyph, this->num_advances - 1)].advance
+    return this->table->longMetric[MIN (glyph, this->num_advances - 1)].advance
 	 + this->var->get_advance_var (glyph, font->coords, font->num_coords); // TODO Optimize?!
   }
 };

src/hb-ot-layout.cc

@@ -279,10 +279,9 @@ hb_ot_layout_get_ligature_carets (hb_font_t      *font,
 				  hb_codepoint_t  glyph,
 				  unsigned int    start_offset,
 				  unsigned int   *caret_count /* IN/OUT */,
-				  int            *caret_array /* OUT */)
+				  hb_position_t  *caret_array /* OUT */)
 {
-  return _get_gdef (font->face).get_lig_carets (font, direction, glyph, start_offset,
-    caret_count, (hb_position_t*) caret_array);
+  return _get_gdef (font->face).get_lig_carets (font, direction, glyph, start_offset, caret_count, caret_array);
 }