Question

I have recently started using the -Wall compiler switch in an attempt to improve the quality of my code. It is giving (correctly) a warning about this little snippet...

    int i;

    for (i = start - 1; i >= 0; i--)
    {
        if (i >= number1.array.size())
        {
            one_value = 0;
        }

because number1.array.size is unsigned (it's the size method on a std::vector). Since the test in the loop is i >= 0, i has to be signed or it doesn't work. It seems I have three choices; to refrain from using -Wall, to ignore the warning, or to introduce an ancillary element...

    int          i;
    unsigned int j;

    for (i = start - 1; i >= 0; i--)
    {
        j = i;

        if (j >= number1.array.size())
        {
            one_value = 0;
        }

None of these seems particularly desirable. Can you suggest any alternative, or make a recommendation as to what I should do in this case?

Was it helpful?

Solution

"Since the test in the loop is i >= 0, i has to be signed or it doesn't work." Just change your test like this:

for(unsigned i = start; i--;) {
    // ...
}

Gives you the same value of i in the loop body.

OTHER TIPS

use 'size_t' for size related comparisons.

size_t i = 0;

First of all, assigning a signed number to an unsigned type could have quite serious consequences (-1 in a signed 32-bit type is 4 294 967 295 in an unsigned), which is one of the reasons for that warning existing. You do the conversion in one place or the other in both solutions, and no matter which of them you use, you would get the same effect by just casting size() to a signed integer.

Something along these lines would eliminate the vulnerability (not checked for correctness)

for(unsigned int i=0;i<start;i++)
{
if(start-i>number1.array.size()) one_value=0;
}

I think :)

You could try:

unsigned int i;

for (i = start; i > 0; )
{

    if (--i >= number1.array.size())
    {
        one_value = 0;
    }

}

This should work the same as your code -
i runs from start to 1 (instead of start-1 to 0), and the test for the array size was changed accordingly.

unsigned int i;

for (i = start; i > 0; i--)
{
    if (i > number1.array.size())
    {
        one_value = 0;
    }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top