CFBitVector: fix multiple correctness bugs

Source/CFBitVector.c

@@ -128,7 +128,9 @@ CFBitVectorGetBitIndex (CFIndex idx)
 CF_INLINE UInt8
 CFBitVectorBitMask (UInt8 mostSig, UInt8 leastSig)
 {
-  return ((0xFF << (7 - leastSig + mostSig)) >> mostSig);
+  /* Truncate to a byte before the right shift: 0xFF << n is computed as an
+     int, so without the cast the high bits survive and the mask is wrong. */
+  return ((UInt8)(0xFF << (7 - leastSig + mostSig)) >> mostSig);
 }
 
 static void
@@ -150,7 +152,7 @@ CFBitVectorOperation (CFBitVectorRef bv, CFRange range,
   /* First byte */
   if (curByte == endByte)
     {
-      mask = CFBitVectorBitMask (startBit, startBit + endBit);
+      mask = CFBitVectorBitMask (startBit, endBit);
       multiByte = false;
     }
   else
@@ -227,7 +229,17 @@ CFBitVectorGetBitAtIndex (CFBitVectorRef bv, CFIndex idx)
 void
 CFBitVectorGetBits (CFBitVectorRef bv, CFRange range, UInt8 *bytes)
 {
-
+  CFIndex i;
+
+  /* Pack the bits in RANGE into BYTES, most-significant bit first, so that
+     bit I of the range becomes bit I of the output (matching the bit order
+     used by CFBitVectorGetBitAtIndex). */
+  memset (bytes, 0, CFBitVectorGetByteCount (range.length));
+  for (i = 0 ; i < range.length ; i++)
+    {
+      if (CFBitVectorGetBitAtIndex (bv, range.location + i))
+        bytes[i >> 3] |= (0x80 >> (i & 7));
+    }
 }
 
 CFIndex
@@ -282,12 +294,12 @@ CFBitVectorGetFirstIndexOfBit (CFBitVectorRef bv, CFRange range, CFBit value)
 {
   CFIndex idx;
 
-  for (idx = range.location ; idx < range.length ; idx++)
+  for (idx = range.location ; idx < range.location + range.length ; idx++)
     {
       if (value == CFBitVectorGetBitAtIndex (bv, idx))
         return idx;
     }
-  
+
   return kCFNotFound;
 }
 
@@ -296,7 +308,7 @@ CFBitVectorGetLastIndexOfBit (CFBitVectorRef bv, CFRange range, CFBit value)
 {
   CFIndex idx;
 
-  for (idx = range.location + range.length ; idx < range.location ; idx--)
+  for (idx = range.location + range.length - 1 ; idx >= range.location ; idx--)
     {
       if (value == CFBitVectorGetBitAtIndex (bv, idx))
         return idx;
@@ -321,8 +333,11 @@ CFBitVectorCreateMutable (CFAllocatorRef alloc, CFIndex capacity)
 
       byteCount = CFBitVectorGetByteCount(capacity);
       new->_bytes = CFAllocatorAllocate (alloc, byteCount, 0);
+      new->_byteCount = byteCount;
+      if (new->_bytes)
+        memset (new->_bytes, 0, byteCount);
     }
-  
+
   return new;
 }
 
@@ -406,10 +421,12 @@ CFBitVectorSetCount (CFMutableBitVectorRef bv, CFIndex count)
 
           newBytes = CFAllocatorAllocate (CFGetAllocator(bv), newByteCount, 0);
           memcpy (newBytes, bv->_bytes, bv->_byteCount);
-
+          memset (newBytes + bv->_byteCount, 0, newByteCount - bv->_byteCount);
+
           CFAllocatorDeallocate (CFGetAllocator(bv), bv->_bytes);
-          
+
           bv->_bytes = newBytes;
+          bv->_byteCount = newByteCount;
           bv->_count = count;
         }
       else

Tests/CFBitVector/general.m

@@ -0,0 +1,41 @@
+#include "CoreFoundation/CFBitVector.h"
+#include "../CFTesting.h"
+
+int main (void)
+{
+  CFMutableBitVectorRef bv;
+  UInt8 out[4];
+
+  bv = CFBitVectorCreateMutable (NULL, 0);
+  CFBitVectorSetCount (bv, 16);
+  PASS_CF(CFBitVectorGetCount(bv) == 16, "Count set to 16.");
+
+  /* Newly grown bits start cleared. */
+  PASS_CF(CFBitVectorGetBitAtIndex(bv, 0) == 0
+       && CFBitVectorGetBitAtIndex(bv, 15) == 0, "New bits are 0.");
+
+  /* SetAllBits must set every bit. */
+  CFBitVectorSetAllBits (bv, 1);
+  PASS_CF(CFBitVectorGetBitAtIndex(bv, 0) == 1
+       && CFBitVectorGetBitAtIndex(bv, 15) == 1, "SetAllBits sets all bits.");
+  CFBitVectorSetAllBits (bv, 0);
+
+  /* Set a couple of bits and search within sub-ranges (location > 0). */
+  CFBitVectorSetBitAtIndex (bv, 5, 1);
+  CFBitVectorSetBitAtIndex (bv, 11, 1);
+  PASS_CF(CFBitVectorGetFirstIndexOfBit(bv, CFRangeMake(3, 10), 1) == 5,
+    "GetFirstIndexOfBit honours the range location.");
+  PASS_CF(CFBitVectorGetLastIndexOfBit(bv, CFRangeMake(0, 16), 1) == 11,
+    "GetLastIndexOfBit returns the last set bit.");
+  PASS_CF(CFBitVectorGetFirstIndexOfBit(bv, CFRangeMake(0, 5), 1) == kCFNotFound,
+    "GetFirstIndexOfBit reports no match when the range has none.");
+
+  /* GetBits packs the requested range, most-significant bit first.
+     Range (4,8) covers bits 4..11; the set bits 5 and 11 map to range
+     offsets 1 and 7 -> 0b01000001 = 0x41. */
+  CFBitVectorGetBits (bv, CFRangeMake(4, 8), out);
+  PASS_CF(out[0] == 0x41, "GetBits packs the requested range.");
+
+  CFRelease (bv);
+  return 0;
+}