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 instead of casting?

Answer 1

One workaround would be to selectively disable that one warning in this special case. GCC has pragma diagnostic ignored "-Wsomething"

// Disable a warning for a block of code:
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wmissing-prototypes"
// ... Some code where the specified warning should be suppressed ...
#pragma GCC diagnostic pop

Recent versions of GCC (I am not actually sure since when, but 4.8.x should support it) show the corresponding -Wsomething option. This is important since most warning options are not set explicitly but en bloc with options like -Wall. An error message would look like this:

readers.c: In function ‘buffered_fullread’:
readers.c:864:11: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
    if(got < sizeof(readbuf)) /* That naturally catches got == 0, too. */

The [-Werror=sign-compare] part tells You You can use "Wsign-compare" for "Wsomething" to suppress the warning.

And of course, You should only do that where it is appropriate (it does not quite help readability), e.g. when exactly the behaviour that the compiler warns about is wanted (or, if You may not introduce bigger changes in the code base).

Answer 2

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 3

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 4

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 5

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 6

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 7

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.