bk commit into 5.1 tree (mats:1.1792)

[Mats Kindahl]

Below is the list of changes that have just been committed into a local
5.1 repository of mats. When mats does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.1792 05/03/21 09:25:52 mats@stripped +1 -0
  Added some compile-time checks.
  Bits are stored big-endian in each byte.
  my_free() does not accept NULL.

  sql/bitvector.h
    1.4 05/03/21 09:25:47 mats@stripped +39 -19
    Added some compile-time checks.
    Bits are stored big-endian in each byte.
    my_free() does not accept NULL.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	mats
# Host:	romeo.kindahl.net
# Root:	/home/bk/bv-mysql-5.1

--- 1.3/sql/bitvector.h	2005-03-17 13:38:03 +01:00
+++ 1.4/sql/bitvector.h	2005-03-21 09:25:47 +01:00
@@ -1,4 +1,6 @@
-/* Copyright (C) 2005 MySQL AB
+/* -*- Mode: C++ -*-
+
+   Copyright (C) 2005 MySQL AB
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -22,11 +24,17 @@
 #include <cstring>
 #include <climits>
 
+// Some compile-time checks to ensure the integrity of the implementation.
 #if CHAR_BIT != 8
 #  error "This implementation is designed for 8-bit bytes!"
 #endif
 
+#ifndef __cplusplus
+#  error "This is not a C header file, it's a C++ header file"
+#endif
+
 namespace {
+    // Defining my own swap, since we cannot use std::swap()
     template <class T> 
     inline void my_swap(T& x, T& y) { 
 	T t(x); x = y; y = t; 
@@ -34,20 +42,28 @@
 }
 
 /*
-  A dynamic bitvector for storing bits.
+  A run-time sized bitvector for storing bits.
+
+  
+  CAVEAT
+
+    This class is not designed to be inherited from, please don't do that.
 
-  Right now, the vector cannot change size. It's only used as a replacement
-  for using an array of bytes and a counter. If you want to change the size of
-  the vector, construct a new bitvector and assign it to the vector, e.g.:
+    Right now, the vector cannot change size. It's only used as a replacement
+    for using an array of bytes and a counter. If you want to change the size
+    of the vector, construct a new bitvector and assign it to the vector,
+    e.g.:
 
-    bitvector new_bv(new_size);
-    old_bv = new_bv;
+      bitvector new_bv(new_size);
+      old_bv = new_bv;
 
-  An alternative is to use the swap member function to replace the instance:
+    An alternative is to use the swap member function to replace the instance:
 
-    bitvector new_bv(new_size);
-    old_bv.swap(new_bv);
+      bitvector new_bv(new_size);
+      old_bv.swap(new_bv);
 
+    The latter may be more efficient on compilers that are not so good at
+    inlining code.
 */
 
 class bitvector {
@@ -69,23 +85,24 @@
     bool operator()(byte* p, byte m) { return *p & m; }
   };
 
+  // Compute the number of bytes required to store 'bits' bits in an array.
   static inline size_t byte_size(size_t bits) { 
     int const byte_bits = sizeof(byte) * CHAR_BIT;
     return (bits + (byte_bits-1)) / byte_bits; 
   }
 
   // Tidy the last byte (by clearing the unused bits) of the bitvector to make
-  // comparison easy.
-  // Here I'm assuming that we're working with 8-bit bytes.
+  // comparison easy.  This code is assuming that we're working with 8-bit
+  // bytes.
   void tidy_last_byte() {
     byte* const last_byte = m_data + bytes() - 1;
 
     // Get the number of unused bits (0..7) in the last byte
     unsigned int const unused = ((8U - (size() & 0x7U)) & 0x7U);
 
-    // Create a mask with the lower 'unused' bits clear and the upper bits
-    // set.
-    unsigned int const mask = ~((1 << unused) - 1);
+    // Create a mask with the upper 'unused' bits clear and the lower 'used'
+    // bits set. The bits within each byte is stored in big-endian order.
+    unsigned int const mask = ((1 << unused) - 1);
 
     // Mask the last byte
     *last_byte &= mask;
@@ -141,7 +158,8 @@
 
   ~bitvector() 
   {
-    my_free(m_data, MYF(0));
+    if (m_data)
+      my_free(m_data, MYF(0));
   }
 
   // Swap the guts of this instance with another instance.
@@ -161,9 +179,9 @@
 
   // Set all bits in the vector
   void set_all() { 
-    // I would really like to use std::fill() here, but I'm not sure it's
+    // I would really like to use std::fill_n() here, but I'm not sure it's
     // allowed.
-    // std::fill(m_data, m_data + bytes(), 255);
+    // std::fill_n(m_data, bytes(), 255);
     memset(m_data, 255, bytes()); 
     tidy_last_byte();
   }
@@ -175,7 +193,7 @@
 
   // Reset (clear) all bits in the vector
   void clear_all() { 
-    // std::fill(m_data, m_data + bytes(), 0);
+    // std::fill_n(m_data, bytes(), 0);
     memset(m_data, 0, bytes()); 
     tidy_last_byte();
   }
@@ -196,6 +214,8 @@
   bool operator==(bitvector const& rhs) const {
     if (size() != rhs.size())
       return false;
+    // This works since I have ensured that the last byte of the array contain
+    // sensible data.
     if (memcmp(data(), rhs.data(), bytes()) != 0)
       return false;
     return true;