CODEWITHSUNDEEP
ccompiler-warningsunsigned
Dadam42
Dadam42

Reputation: 65

Confused about understanding and silencing a -Wsign-compare gcc's warning

Here is a function i am trying to compile :

static ssize_t  output(t_out_buffer *buf, char const *src, size_t size)
{
        size_t  osize;

        osize = size;
        while ((size > 0)
               && (size -= write(buf->fd, src, size) < osize))
        {
                src += osize - size;
                buf->count +=  osize - size;
                osize = size;
        }
        if (osize < size)
                return (T_OUT_BUFFER_ERROR);
        else
                return (buf->count);
}

And gcc's complain :

t_out_buffer.c:11:42: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
    && (size -= write(buf->fd, src, size) < osize))
                                          ^

I assumed that as size is unsigned, size -= whateverintiwant would be unsigned as osize is unsigned too. I assume now that I was wrong but I do not really understand why. Furthermore, can you give me hints to silence this ?

Upvotes: 3

Views: 396

Answers (2)

dbush
dbush

Reputation: 223927

This expression isn't doing what you think:

(size -= write(buf->fd, src, size) < osize)

The less-than operator < has higher precedence than the compound assignment operator -=. So the above parses as:

(size -= (write(buf->fd, src, size) < osize))

So this compares the output of write which is of type ssize_t with osize which is of type size_t. This is where the signed/unsigned comparison happens. The result of this comparison is then subtraced from size, so it will only ever decrease by 1 at a time.

Add parenthesis around the assignment:

((size -= write(buf->fd, src, size)) < osize)

And the warning will go away as you are now comparing a size_t with a size_t.

There's another problem however. If write returns -1 then you'll be subtracting the value, i.e. adding 1, if it fails.

You should refactor so that you do the read inside the loop and only add the result if it succeeds.

    while (size > 0) {
    {
            ssize_t rval = write(buf->fd, src, size);
            if (rval == -1) {
                return T_OUT_BUFFER_ERROR;
            }
            size -= rval;
            src += rval;
            buf->count +=  rval;
    }

Upvotes: 8

Michele Dorigatti
Michele Dorigatti

Reputation: 817

I think you have an operator precedence problem. Your code is interpreted:

&& (size -= (write(buf->fd, src, size) < osize)))

So, you can fix it by forcing the desired precedence:

&& ((size -= write(buf->fd, src, size)) < osize))

The warning is helping you by pointing out a problem, you should never silence a warning if you don't fully understand it.
Moreover, it is bad practice to use side effects inside a condition. Your code would be easier to understand and maintain if you moved the assignment operator -= outside the condition.

Upvotes: 3

Related Questions