Why is FxCop warning about an overflow (CA2233) in this C# code?

Question

I have the following function to get an int from a high-byte and a low-byte:

public static int FromBytes(byte high, byte low)
{
    return high * (byte.MaxValue + 1) + low;
}

When I analyze the assembly with FxCop, I get the following critical warning:

CA2233: OperationsShouldNotOverflow
Arithmetic operations should not be done without first validating the operands to prevent overflow.

I can't see how this could possibly overflow, so I am just assuming FxCop is being overzealous.
Am I missing something? And what steps could be taken to correct what I have (or at least make the FxCop warning go away!)?

Your bet is wrong. His code cannot cause an overflow since byte.MaxValue would ALWAYS be implicitly converted to an int before the addition step took place. -- Anytime a method performs an arithmetic operation and does not validate the operands beforehand (to prevent an overflow) you will get CA2233. There are plenty of examples on how to fix this on MSDN at http://msdn.microsoft.com/en-us/library/ms182354.aspx — BrainSlugs83, Jan 09 '12 at 04:43

score 5 · Answer 1 · answered Apr 15 '10 at 01:36

5

It is doing them as byte calculations.

Try this

return (int)high * ((int)byte.MaxValue + 1) + (int)low;

answered Apr 15 '10 at 01:36

Daniel A. White

187,200
47
362
445

I was just about to write that! Nice and Fast. :) – Joshua Apr 15 '10 at 01:39
1

-1! (if I could!) This is incorrect. You do not need to cast byte.MaxValue as an integer -- when you add an integer and a byte -- the byte is AUTOMATICALLY cast to an integer -- this is the whole point of an implicit conversion. For proof, notice: public static int GetValue() { return Byte.MaxValue + 1 } returns the value 256. Byte + Int = Int. Also, please note that the code you provided would not make a CA2233 FxCop warning go away. Simply (i + 1) (where i is an int) will cause this warning. – BrainSlugs83 Jan 09 '12 at 04:40

score 4 · Answer 2 · edited Apr 15 '10 at 02:19

4

Byte addition and mult results are ints. The max value here is 65535 which won't overflow an int. Just surpress the error.

byte a = 1;
byte b = 2;
object obj = a + b

obj has type int

Try this:

        byte high = 255;
        byte low = 255;
        checked
        {
            int b = high * (byte.MaxValue + 1) + low;   
        }

No problem.

or try this

edited Apr 15 '10 at 02:19

John Saunders

160,644
26
247
397

answered Apr 15 '10 at 01:54

csaam

1,349
9
9

score 3 · Answer 3 · answered Apr 15 '10 at 01:51

Here's 2 ways that it finally stopped whining about CA2233 for me:

    public static int FromBytes(byte high, byte low)
    {
        int h = high;
        return h * (byte.MaxValue + 1) + low;
    }

    public static int FromBytes2(byte high, byte low)
    {
        unchecked
        {
            return high * (byte.MaxValue + 1) + low;
        }
    }

I think it might be a bug in the rule.

score 3 · Accepted Answer · edited May 23 '17 at 11:55

3

As Daniel A. White pointed out, you get the message because "(byte.MaxValue + 1)" overflows a byte.

But instead of casting and multiplying, I would simply shift the bits as done in the code below:

public static int FromBytes(byte high, byte low) {
    return high << 8 | low;
}

As a side effect, this code will probably perform better. I haven't checked the resulting IL or x86 to see if the compiler and/or the JITter are smart enough to optimize the original expression.

edited May 23 '17 at 11:55

Community

1
1

answered May 05 '10 at 20:59

Alfred Myers

6,384
1
40
68

4

Again, byte.MaxValue + 1 does not overflow byte. He gets the message because he does not validate his operands. It goes away for your code since your code does not perform any arithmetic, only binary logic. See http://msdn.microsoft.com/en-us/library/ms182354.aspx for more details. – BrainSlugs83 Jan 09 '12 at 04:45

Why is FxCop warning about an overflow (CA2233) in this C# code?

4 Answers4

Linked