[Spice-devel] [PATCH 2/2] Eliminate signed/unsigned warning

[Christophe de Dinechin]

> On 19 Feb 2018, at 13:24, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> On Fri, Feb 16, 2018 at 04:23:06PM +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin <dinechin at redhat.com>
>> 
>> Without this, GCC complains about signed / unsigned comparisons:
>> 
>> mjpeg-fallback.cpp:121:24: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
>>     if (win_info.width != last_width || win_info.height != last_height) {
>>         ~~~~~~~~~~~~~~~^~~~~~~~~~~~~
> 
> Are you getting this using the default CXXFLAGS?

No, this was a side effect of tracking some other warning.


> Here I seem to be getting
> -Wno-sign-compare by default.

Why is this silenced? There aren’t that many of them, and implicit int promotion makes some scenarios risky, e.g signed byte vs unsigned byte.

Hmm, I checked, and the “signed-compare” of GCC does not warn in the dangerous cases, only in the harmless ones… Weird.

No warning for this, where the result is “surprisingly” false because of int promotion (comparison is false), even with -Wall, -Wextra or -Wpedantic
    signed char i = -3;
    unsigned char j = -3;
    printf("i=%x j=%x i==j=%d\n", i, j, i==j);

But a warning for this where the result is true as expected:
    signed char i = -3;
    unsigned char j = -3;
    printf("i=%x j=%x i==j=%d\n", i, j, i==j);

Really strange. Well, if that’s how the option behaves, then -Wno-sign-compare seems harmless.

But does anybody understand the rationale for this? I’m a bit puzzled.

> 
> Christophe
> 
>> 
>> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
>> ---
>> src/mjpeg-fallback.cpp        | 2 +-
>> src/spice-streaming-agent.cpp | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
>> index 634864f..53804d9 100644
>> --- a/src/mjpeg-fallback.cpp
>> +++ b/src/mjpeg-fallback.cpp
>> @@ -57,7 +57,7 @@ private:
>>     std::vector<uint8_t> frame;
>> 
>>     // last frame sizes
>> -    uint32_t last_width = ~0u, last_height = ~0u;
>> +    int last_width = ~0u, last_height = ~0u;
>>     // last time before capture
>>     uint64_t last_time = 0;
>> };
>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>> index 4ec5e42..27b26a4 100644
>> --- a/src/spice-streaming-agent.cpp
>> +++ b/src/spice-streaming-agent.cpp
>> @@ -106,7 +106,7 @@ static int read_command_from_device(void)
>>         return 0; // return -1;
>>     }
>>     n = read(streamfd, &msg, hdr.size);
>> -    if (n != hdr.size) {
>> +    if (n != (int) hdr.size) {
>>         syslog(LOG_WARNING,
>>                "read command from device FAILED -- read %d expected %d\n",
>>                n, hdr.size);
>> -- 
>> 2.13.5 (Apple Git-94)
>> 
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel