bitmask: Fix broken invert_range() implementation

The speed-up in 7da1f8a was wrong in certain conditions, even though it didn't trigger the existing testsuite. New testcase /bitmask/invert_range_hardcoded included.
raja651 · Sep 7, 2015 · c8c666c · c8c666c
1 parent f5fe1e3
commit c8c666c
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 12 deletions.
diff --git a/gtk/gtkallocatedbitmask.c b/gtk/gtkallocatedbitmask.c
@@ -25,7 +25,7 @@
 
 #define VALUE_SIZE_BITS (sizeof (VALUE_TYPE) * 8)
 #define VALUE_BIT(idx) (((VALUE_TYPE) 1) << (idx))
-#define ALL_BITS G_MAXSIZE
+#define ALL_BITS (~((VALUE_TYPE) 0))
 
 struct _GtkBitmask {
   gsize len;
@@ -301,17 +301,11 @@ _gtk_allocated_bitmask_invert_range (GtkBitmask *mask,
   if (end_word >= mask->len)
     mask = gtk_allocated_bitmask_resize (mask, end_word + 1);
 
-  if (start_word == end_word)
-    {
-      mask->data[start_word] ^= (ALL_BITS >> (end_bit - start_bit)) << start_bit;
-    }
-  else
-    {
-      mask->data[start_word] ^= ALL_BITS << start_bit;
-      for (i = start_word + 1; i < end_word; i++)
-        mask->data[i] ^= ALL_BITS;
-      mask->data[end_word] ^= ALL_BITS >> (VALUE_SIZE_BITS - end_bit);
-    }
+  for (i = start_word; i <= end_word; i++)
+    mask->data[i] ^= ALL_BITS;
+  mask->data[start_word] ^= (((VALUE_TYPE) 1) << start_bit) - 1;
+  if (end_bit != 63)
+  mask->data[end_word] ^= ALL_BITS << (end_bit + 1);
 
   return gtk_allocated_bitmask_shrink (mask);
 }

diff --git a/testsuite/gtk/bitmask.c b/testsuite/gtk/bitmask.c
@@ -333,6 +333,49 @@ test_subtract_hardcoded (void)
   _b = _tmp; \
 }G_STMT_END
 
+static void
+test_invert_range_hardcoded (void)
+{
+  guint t, l, r, i;
+  gsize r_len, l_len, ref_len;
+  char *ref_str;
+  GtkBitmask *bitmask, *ref;
+
+  for (t = 0; t < G_N_ELEMENTS (tests); t++)
+    {
+      for (l = 0; l < G_N_ELEMENTS (tests); l++)
+        {
+          l_len = strlen (tests[l]);
+
+          for (r = 0; r < G_N_ELEMENTS (tests); r++)
+            {
+              r_len = strlen (tests[r]);
+              if (r_len < l_len)
+                continue;
+
+              ref_len = MAX (r_len, strlen (tests[t]));
+              ref_str = g_strdup_printf ("%*s", (int) ref_len, tests[t]);
+              for (i = 0; i < ref_len && ref_str[i] == ' '; i++)
+                ref_str[i] = '0';
+              for (i = l_len - 1; i < r_len; i++)
+                {
+                  ref_str[ref_len-i-1] = ref_str[ref_len-i-1] == '0' ? '1' : '0';
+                }
+              ref = gtk_bitmask_new_parse (ref_str);
+              g_free (ref_str);
+
+              bitmask = gtk_bitmask_new_parse (tests[t]);
+              bitmask = _gtk_bitmask_invert_range (bitmask, l_len - 1, r_len);
+
+              assert_cmpmasks (bitmask, ref);
+
+              _gtk_bitmask_free (bitmask);
+              _gtk_bitmask_free (ref);
+            }
+        }
+    }
+}
+
 static void
 test_invert_range (void)
 {
@@ -424,6 +467,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/bitmask/intersect_hardcoded", test_intersect_hardcoded);
   g_test_add_func ("/bitmask/subtract_hardcoded", test_subtract_hardcoded);
   g_test_add_func ("/bitmask/invert_range", test_invert_range);
+  g_test_add_func ("/bitmask/invert_range_hardcoded", test_invert_range_hardcoded);
 
   result = g_test_run ();