Code Analysis, Overflow Danger, & Bitwise Operations

Recently, I had turned on Code Analysis for a project of mine and started working through the things it had found. I came upon a situation where I had coded the following snippet many months ago and had never reviewed it:

protected static byte ConvertNybble(byte nybble)

        **...  
        ****return** (**byte**)(nybble **+** 240);

I had violoated rule CA2233 which states that there is a possible overflow situation in the code block and that it should be remedied. Now that I had a chance to review it a second time along with the warning from Code Analysis, it was as clear as day! By adding 240 to this Byte value, I could easily blow past the upper limit of 255 (0 through 255 being the complete range for what can be stored in 8 bits).

Of course, the entire point of what I was doing here was that I wanted to fill the upper nybble (the first four bits) with 1's and the bytes that I was sending the this method were all bytes that were less than 16 (the first four bits were all 0's) so the overflow would never happen because 240 is 1111 0000 in binary.

However, the safer, and slighly more economical way for me to accomplish this, and certainly more in line with my intent, would be to use a bitwise XOR BITOR on the byte and 0xF0 (hex for 240).

That way in case a byte higher than 15 got passed into the method, it would not affect the operation since XOR BITOR with 0xF0 as below:

return (byte)(nybble | 0xF0);

So, if you haven't done it already, run Code Analysis on your projects. You just might be suprised on the little details that you learn about your code or discover you overlooked in the rush to meet feature requirements.

Per Mark Seeman's advice, I would turn off XML comment warnings and code analysis until you are either ready to solve them or at least in your Release build configuration. Otherwise, you'll likely find yourself swiming in a sea of compiler warnings that you might not be inclined to solve immediately and thus you might miss important ones that are true compiler warnings versus Code Analysis flags.

[Update: I must make a correction here, I had mistakening referred to the operation about as an XOR, when in fact, it is a BITOR, or Inclusive OR. The code and syntaxed used was correct, however, I had mistakeningly called the operation the wrong thing.]