| Giorgos Keramidas 2008-01-21, 7:28 pm |
| On Mon, 21 Jan 2008 08:08:09 +0100, Lars Uffmann <aral@nurfuerspam.de> wrote:
> Rainer Weikusat wrote:
>
> Of course, thanks - I was missing the obvious :) - I even had the
> comment "returns 2^((bit - 1) % BITS_PER_INT) for a set bit" in my
> source documentation for the reading function...
>
> Bitmap thing optimized now... in case anyone wants to use it.
An interesting exercise in the NIH syndrome[1], but there are still
many `problems' with this implementation.
[1] http://en.wikipedia.org/wiki/Not_Invented_Here
> #ifndef BITMAP_H_
> #define BITMAP_H_
>
> #include <limits.h> /* for CHAR_BIT */
>
> const unsigned int BITS_PER_INT = sizeof(int) * CHAR_BIT;
Why isn't `unsigned char' sufficient as a backend store? This way you
wouldn't have to invent a new macro for BITS_PER_INT, because CHAR_BIT
would be enough for what you are currently doing.
> class Bitmap
> {
> protected:
> unsigned int size; // total amount of bits mapped
> unsigned int *bitmap; // data area
> public:
> Bitmap(unsigned int); // construct with parameter size in bits
> virtual ~Bitmap();
>
> void resetBitmap(); // unset all bits
> int set (unsigned int); // set a bit
> int get (unsigned int); // get a bit
> };
>
> #endif /*BITMAP_H_*/
There is a standard type for storing the size of objects: size_t.
Why do you have to go through all the gymnastics of error codes, just to
use `unsigned int' as the type?
> -------------------------
> #include "Bitmap.h"
> #include <cstring> // for memset
>
> Bitmap::Bitmap (unsigned int bits)
> {
> size = bits;
> bitmap = new unsigned int [(size + BITS_PER_INT - 1) /
> BITS_PER_INT]; // allocate bitmap memory
> resetBitmap();
> }
What if new() fails?
> void Bitmap::resetBitmap()
> {
> memset (bitmap, 0, sizeof (bitmap)); // initalize bitmap with 0
> }
If new() fails in Bitmap::Bitmap() you are trying to overwrite the
memory pointed by a NULL pointer here and in all the places below where
bitmap[] is used as a vector.
There's a _very_ good reason why I suggested to reuse tested code in the
original post which referenced the BSD <sys/bitstring.h> header file.
All the bugs in the implementation above (including the horrible
formatting, which may be an artifact of the newsreader used to post)
fall in the category of "This is why I initially suggested bitstring.h".
|