C: how can I fix warnings like: "comparison between signed and unsigned"

Question

I have been advised to use the following options with GCC, as it helps to avoid a lot of common errors. It turns on a bunch of warnings and -Werror turns them into errors.

gcc -pedantic -W -Wall -Wextra -Wshadow -Wstrict-overflow=5 -Wwrite-strings -std=c99 -Werror

Given the following test code:

#include <stdio.h>

int main(void)
{
    int arr[8]={0,10,20,30,40,50,60,70};
    int x;

    printf("sizeof(arr): %d\n", sizeof(arr));
    printf("sizeof(int): %d\n", sizeof(int));

    for(x=0;x<sizeof(arr) / sizeof(int);x++)
    {
     printf("%d\n",arr[x]);
    }

    return 0;
}

I get this:

test.c:11: error: comparison between signed and unsigned

I know that one way I can fix this is turning off the warnings, but they haven't made me use these settings to turn them off in the end.

Another way is to cast the stuff, but I have been told that casting is deprecated.

Also, I could make x into an unsigned int:

unsigned x;

But it doesn't solve the general problem when I have to compare signed values with unsigned values using these compiler options. Is there an cleaner way insted of casting?

Answer 1

test.c:11: error: comparison between signed and unsigned

You could declare x as an unsigned int, since size_t is unsigned

EDIT:

If you don't want to cast, and don't want to declare it as unsigned, i don't think there's much to do.

Maybe bitwise operations are a way of solving it, removing the sign bit. I have to say, IMO its very questionable though.

Answer 2

Replace

int x;
/* ... */
for(x=0;x<sizeof(arr) / sizeof(int);x++)

by

for(size_t x=0;x<sizeof(arr) / sizeof(int);x++)

But it doesn't solve the general problem when I have to compare signed values with unsigned values using these compiler options. Is there an cleaner way insted of casting?

In such cases, try to figure out if the signed number can ever have a value which will cause overflow. If not, you can ignore the warning. Otherwise a cast to the unsigned type (if this is the same size or bigger than the signed component) is good enough.

Answer 3

This really depends on the data type. It is possible to avoid this by implicitly casting the values to a type which contains a superset of all the values available in the signed and unsigned type. For instance you could use a 64 bit signed value to compare an unsigned 32 bit and a signed 32 bit value.

However this is a corner case and you need to do operations like verify the size of your types. Your best solution is to use the same type for both operands.

If you must cast, do consider that you could be causing an overflow and consider if that is important to your application or not.

Answer 4

The crux of the matter is that comparing signed and unsigned values admits some weird cases. Consider, for instance, what happens in the unsigned array length is larger than the maximum that can be represented by a signed int. The signed counter overflow (remaining "less than" the array size), and you start addressing memory you didn't mean to...

The compiler generates a warning to make sure that you're thinking about them. Using -Werror promotes that warning to an error and stops the compilation.

Either be rigorous about choosing the signedeness of your types, or cast the trouble away when you're sure it doesn't apply, or get rid of -Werror and make it a policy to address all warnings with a fix or an explanation...

Answer 5

We suppress this warning in our Visual Studio compiles, since it happens a lot and almost never means anything significant. Of course, not all coding standards allow that.

You can make types agree (declaring variables to be size_t or unsigned int instead of int, for example), or you can cast, or you can change your compilation line. That's about it.

Answer 6

Regardless of the casting deprecation dilemma, I'd still suggest separating out the logic from the for loop.

int range = (int)(sizeof(arr) / sizeof(int));
int x;

for(x = 0; x < range; x++)
{
    printf("%d\n", arr[x]);
}

Although this uses casting which you said was deprecated, it clears up the location where the casting is taking place. In general, I advise against cramming a lot of logic into your for loop declarations. Especially in your case where you're using size_t division, which (because it is integer division) could have the possibility of truncating the answer. The for loop declaration should be clean, and should generate no errors. Your casting is occuring in a different location which means if you want to change the way you are creating the range, you don't have to muck about making the for declaration even longer.

Answer 7

one option would be the additional flag "-Wno-sign-compare" :)