Overflow check does not work

Question

I have following code that has a multiplication of a number with 60000 to convert minutes to milliseconds. I have implemented a overflow check as shown below. Still I am getting the following code analysis warring. How to overcome this warning without suppressing it?

Warning: CA2233: Correct the potential overflow in the operation 'sessionExpiryValueInMinutes*60' in 'ApplicationSessionDAL.IsSessionExpired(short)'

Note: TimeSpan.TotalMilliseconds Property is of double datatype

CODE

    public void IsSessionExpired(Int16 sessionExpiryValueInMinutes)
    {

        if (sessionExpiryValueInMinutes > (double.MaxValue) / 60000)
        {
            //Overflow check
            throw new ArgumentOutOfRangeException("sessionExpiryValueInMinutes");
        }
        else
        {
            //int milliSecondsValue  = sessionExpiryValueInMinutes * 60 * 1000;

            DateTime lastAccessTime = new DateTime(2013, 1, 1);
            TimeSpan elapsedTime = (DateTime.Now - lastAccessTime);
            if (elapsedTime.TotalMilliseconds > (sessionExpiryValueInMinutes * 60 * 1000))
            {
                bool isTimeExpired = true;
            }

        }

    }

REFERENCES

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

FXCop can't tell that you have checked for the overflow. It isn't _that_ smart. — Oded, Jan 17 '13 at 10:55
@Oded Certain times it works, for example, while addition. When we specify the range check it stop complaining the warning. I am looking for such a solution — LCJ, Jan 17 '13 at 10:56
Should you be checking Int16.MaxValue instead of double.MaxValue? — Kevin Brydon, Jan 17 '13 at 10:59
@KevinBrydon As I given in the question, TimeSpan.TotalMilliseconds Property is of `double` datatype. — LCJ, Jan 17 '13 at 10:59
FYI I'm not getting any warning for that code with all Code Analysis warnings turned on (CA2233 in particular) with Visual Studio 2012 with VS2012 Update 1 installed. — Matthew Watson, Jan 17 '13 at 11:00
What is the purpose of using `Int16`? Just use `Int32/int` and be done with it. — leppie, Jan 17 '13 at 11:09

score 5 · Accepted Answer · answered Jan 17 '13 at 11:00

You may wrap your computation in a checked block. That way, the program will explicitly throw a System.OverflowException which you can here catch to do what you want. And since you want to throw an exception anyway, in your particular case, you don't need to do anything else.

Example:

checked 
{
    if (elapsedTime.TotalMilliseconds > (sessionExpiryValueInMinutes * 60 * 1000))
    {
        bool isTimeExpired = true;
    }
}

And @Oded is right, FxCop cannot be always that smart.

score 0 · Answer 2 · answered Jan 17 '13 at 11:07

I am not sure, but I suspect that it means that the result of (sessionExpiryValueInMinutes * 60 * 1000) will never hold into an Int16.

I see another issue with your code: I'm almost certain that sessionExpiryValueInMinutes, being an Int16, will never ever be greater than (double.MaxValue)/60000.

Overflow check does not work

2 Answers2