Safe countdown loop

151 Views Asked by At

The following code will produce a segfault due to integer overflow when i reaches -1. If I change "unsigned int i" to "char i", then it will work fine but will generate a compiler warning "array subscript has type 'char'". Declaring it as "int i" will work fine and there won't be compiler warning but it feels like there should be. After all int is signed as well and could go also negative. My question is, is there a safe, elegant, idiomatic way to write such loops in C?

#include <stdio.h>

int main() {
    unsigned int i;
    char a[10] = {0};

    for (i = 9; i >= 0; i--) {
        printf("a[%d]: %d\n", i, a[i]);
    }

    return 0;
}
3

There are 3 best solutions below

0
chqrlie On BEST ANSWER

The loop in your code indeed does not work as the test i >= 0 is always true. Compiling with extra warnings will spot this problem.

To avoid this problem, i should be initialized to 10, the test should be i > 0 and i should be decremented at the start of each iteration instead of after it:

    for (unsigned i = 10; i > 0;) {
        i--;
        printf("a[%d]: %d\n", i, a[i]);
    }

Combining the test and the decrement produces the classic down loop that works for both signed and unsigned index types:

#include <stdio.h>

int main(void) {
    char a[10] = { 0 };

    for (unsigned i = sizeof(a) / sizeof(*a); i-- > 0;) {
        printf("a[%u]: %d\n", i, a[i]);
    }

    return 0;
}

The test i-- > 0 is only false for i == 0, but i is decremented as a side effect, so the first iteration uses the value 9 inside the loop body, the second uses 8... and the last uses 0, the value of i after the last decrement. The next next will evaluate to false and leave i with the value UINT_MAX.

Another advantage of this technique is i is initialized to the number of elements, 10, which is also the number of iterations, not 9 as in the question code.


Note also that i-- > 0 can be written i --> 0 as explained in this popular question. While i-- > 0 is idiomatic in C, i --> 0 is not. Whether one is more elegant than the other is a matter of opinion

17
Vlad from Moscow On

For starters according to the C Standard the function main without parameters shall be declared like

int main( void )

As for the loop then 1) do not use magic numbers like 9 and 2) in general use indices of type size_t and at last 3) declare variables in minimum scope where they are used..

Also objects of an unsigned integer type can not have negative values. For example if you will write

unsigned int i = -1;

then the variable i will have a very big positive value.

Try for example this code snippet

unsigned int i = -1;
printf( "i = %u\n", i );

And even the type char can behave as type unsigned char depending on compiler options.

So when the variable i having an unsigned integer type and value 0 is decremented it gets a big positive value. As a result there can be access outside the array or you can have an infinite loop.

Here is your updated program

#include <stdio.h>

int main( void ) 
{
    enum { N = 10 };
    char a[N] = {0};

    for ( size_t i = N; i-- > 0; ) 
    {
        printf( "a[%zu]: %c\n", i, a[i] );
    }

    return 0;
}

The loop can be written also like

for ( size_t i = N; i > 0; i-- ) 
{
    printf( "a[%zu]: %c\n", i - 1, a[i -1] );
}
0
Luis Colorado On

Well, that's correct. What you have written

    unsigned int i;
    char a[10] = {0};

    for (i = 9; i >= 0; i--) {
        printf("a[%d]: %d\n", i, a[i]);
    }

of course, if i is unsigned, then the predicate i >= 0 is ALWAYS true, so you will run on trouble (not by the overflow, but because you use i to access the a array.

Had you written (as unsigned numbers over/underflow in a predictable and precise manner)

    for (i = 9; i <= 9; i--) {
        printf("a[%d]: %d\n", i, a[i]);
    }