c++arraysunsigned-integerstatic-code-analysispvs-studio

Dealing with unsigned integers


I know that unsigned integers are infamous and generally avoided by C++ devs. I have a class with two int member variables that should not contain negative values:

.
.
.
private:
    int m_Y_AxisLen;
    int m_X_AxisLen;
.
.
.

I have designed the logic of the member functions in a way that prevents any input of negative numbers. So I have made sure that those two members won't be assigned negative values.
But this also brings up some warnings when I use PVS-Studio. For example here:

for ( int row = 0; row < getY_AxisLen( ); ++row )
{
    for ( int column = 0; column < getX_AxisLen( ) - 1; ++column )
    {
        if ( m_characterMatrix[ row ][ column ] == getFillCharacter( ) )
        {
            m_characterMatrix[ row ][ column ] = fillCharacter;
        }
    }
}

PVS-Studio blames me for the indices row and column not being of memsize type. It probably means that I should have used std::size_t row and std::size_t column??
But if I had done it that way then it would still complain and say that comparing unsigned integral type with getY_AxisLen( ) (which returns an int) is dangerous.
So this is the reason that I want to rewrite parts of my class to switch to this:

private:
    uint32_t m_Y_AxisLen;
    uint32_t m_X_AxisLen;

I am humbly looking for insights and advice from professionals who have dealt with these kinds of problems before. What would be your approach when it comes to these issues?


Solution

  • A lot of those "you shouldn't use unsigned integers" are just basically scared that you will mix up signed integers and unsigned ones, causing a wrap-around, or to avoid complex integer promotion rules.

    But in your code, I see no reason not to use std::uint32_t and std::size_t, because m_X_AxisLen and m_Y_AxisLen should not contain negative values, and using std::uint32_t and std::size_t makes a lot more sense here:

    So, I suggest changing m_X_AxisLen and m_Y_AxisLen to:

    std::size_t m_Y_AxisLen;
    std::size_t m_X_AxisLen; // for consistency
    

    Change row and column to

    std::size_t row = 0;
    // and 
    std::size_t column = 0;
    

    Make getX_AxisLen( ) returns an std::size_t

    And make the for loop:

    for ( int column = 0; column < getX_AxisLen( ) - 1; ++column )
    

    to:

    for ( int column = 0; column + 1 < getX_AxisLen( ); ++column )
    

    Because if getX_AxisLen() returns 0, getX_AxisLen( ) - 1 will cause a wrap-around.

    Basically, use the thing that makes sense. If a value cannot be negative, use the unsigned type.