bitstate() bug.

[stonedzealot]

This is really weird, I think. I've got this function, bitstate() that returns the current state of a certain bit (wow, imagine that). It looks like this

Code: Select all

int bitstate(int value, char bit, char status)
{
   return ((value & (1 << bit)) >> bit == status);
}

Up until now it's worked perfectly. But this is the first time I've tried to take the status of the 31st bit of a value. For example, this code:

Code: Select all

bitstate(0xFFF00000, 31, 1);

returns 0... which doesn't make any sense because the highest 12 bits of 0xFFF00000 should all register as ON. If I take the code out of the function, however, it works fine:

Code: Select all

(0xFFF00000 & (1 << 31)) >> 31 == ON;

returns 1, like it's supposed to. I realize that this means there's a bug in the function, but... how?

[Candy]

Use an unsigned int. That's probably the cause of your problem, since bit 31 is the sign bit and the rest is the same.

[Solar]

The "trick" is the right-shift (... >> bit).

1 << 31 is 0x80000000. In unsigned arithmetic, right-shifting that value means that the sign bit will "stick", i.e. shifting that >> 31 leaves you with 0xFFFFFFFF - or -1, which is != 1, and thus the function returns 0.

A bit of printing intermediate values to screen (or reading up the details of the operators in use) quite often does the trick.

[Schol-R-LEA]

I could also mention that using a char for [tt]status[/tt] is both misleading (you are using it for a 1-byte unsigned numeric value, not as a [tt]char[/tt]) and not type safe (especially since some compilers use signed [tt]char[/tt]s by default). You be better to use an [tt]enum[/tt]:

Code: Select all

typedef enum {OFF = 0, ON = 1} BitStatus;

The compiler should be smart enough to use the smallest bit width to fit a given enum type, so it shouldn't be any less efficient. You will probably also want the return value to be the same type as well.

Similarly, for the [tt]bit[/tt] argument, you might want to use an [tt]uint8_t[/tt] (defined in [tt]<stdint.h>[/tt], though you usually want to use <inttypes.h>, which includes it; if your compiler doesn't support C99, you can define it with a [tt]typedef[/tt]). It won't change the behavior, but it will make it clear that it's not actually a character value. I know it sounds anal-retentive, but it really can make a difference when you're reading code you haven't looked at in a while.

[Solar]

[tt]uint8_t[/tt] (defined in [tt]<stdint.h>[/tt], though you usually want to use <inttypes.h>, which includes it)

Erm... actually, no standard header is allowed to include some other standard header. I haven't checked what GCC actually does in regards to these two, but if inttypes.h includes stdint.h, that's non-conformig behaviour (i.e., a bug).

While we're at anal-retentive.

[Schol-R-LEA]

[me=Schol-R-LEA]smacks forehead[/me]

OK, that was careless of me. I was basing this on the information in this page, which is the Open Group UNIX standards, not the C language standards. Checking around, I gather that <inttypes.h> isn't part of the C standard at all. Sorry.

[Perica]

..

[Solar]

Aaaaaaaaahhhhhh!

Jesus, how could I have missed that one? ::) :-[

I mean, due to my work on PDCLib, I have a copy of the C99 docs with me at all times. But I was too lazy to fire up the PDA and to a search, so I just used www.dinkumware.com to look it up... and they don't mention that inttypes.h includes stdint.h!

OK, that's it, Dinkumware just died as a valid reference for my PDCLib work. Sheesh...

[df]

ouch. dont write dinukware off, i dont know of any better sites that offer what they do for libc info...

[Solar]

ouch. dont write dinukware off, i dont know of any better sites that offer what they do for libc info...

As I said, I have the standard docs here, conveniently converted from PDF to text so I can do full-text searches and copy&paste on my PDA. I used Dinkumware as a kind of "secondary reference", but an error as big as this one will make me think twice from now on...