1

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;
}
Improve this question
13
  • It should be signed, otherwise it's a forever loop (unsigned are always >=0 and wraparaound overflow/underflow is fine on them). Going from 0 to -1 isn't underflow/overflow, going left of INT_MIN or right of INT_MAX is. Commented Nov 20, 2023 at 21:01
  • 4
    To discuss C clearly, you should learn and use proper terminology. i cannot overflow or reach −1. Unsigned arithmetic is defined to wrap, so, when i is zero, i-- wraps it around to the maximum representable value, UINT_MAX. This is different from overflow in the meaning of the C standard. So you should say that i wraps (not overflows) and becomes large (not −1), and then there is a segment fault because a[i] attempts to access an element out of bounds (not directly because i overflowed or wrapped). Commented Nov 20, 2023 at 21:03
  • Check your compiler options. Your compiler should issue a warning when i is an unsigned int because in this case, the condition i >= 0 is always true and the loops never ends. Commented Nov 20, 2023 at 21:04
  • Re “My question is, is there a safe, elegant, idiomatic way to write such loops in C?”: I am commenting rather than answering because I would want to consider or research further, but I would not say there is a single idiomatic solution. This situation is a nuisance, and it is often written around in various ways. We might discuss using i != -1, as it works with both signed types and unsigned types at least as wide as int (because the −1 will be converted to the maximum value). That could be reasonable as an idiomatic solution except it fails for narrower unsigned types. Commented Nov 20, 2023 at 21:06
  • 2
    @EricPostpischil The OP said the compiler issued a warning when they changed from unsigned int to char. Commented Nov 20, 2023 at 21:15

3 Answers 3

Reset to default
5

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

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

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] );
}
Improve this answer

17 Comments

There's lots of good recommendations in this answer, but when you actually got to answering the specific problem you didn't explain the solution at all.
@Barmar May I ignore your comment?:)
Re “For starters according to the C Standard the function main without parameters shall be declared like”: No, that is not what the C standard says. Besides the (void) and (int argc, char *argv[]) and implementation-defined manner, it allows “or equivalent”, and int main() in a definition is equivalent, as calling a function defined with int main() using a call without arguments, main(), is defined to work.
@EricPostpischil From the C23 Standard "5.1.2.2.1 Program startup 1 The function called at program startup is named main. The implementation declares no prototype for this function. It shall be defined with a return type of int and with no parameters: int main(void) { /* ... */ }" Implementation defined declaration is not a standard declaration.
@chqrlie I am sorry. I have no dollars. I am unemployed.:)
|
-1

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]);
    }
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.