0

I have a socket communication program. The protocol is that any error in writing is fatal, so the connection should be closed. My I/O code looks like this:

auto const toWrite = buf.size() * sizeof(buf[0]);
auto nWritten = ::write(fd, buf.data, toWrite);

if (toWrite != nWritten)
{
    closeTheSocket();
}

This code gives warning: comparison between signed and unsigned integer expressions on the boolean test.

I understand the evils of greater/less comparisons on signed vs unsigned, but it's unavoidable here. The signature for the ::write system call is

  #include <unistd.h>

  ssize_t write(int fd, const void *buf, size_t count);

In other words, my toWrite variable is properly unsigned and the returned nWritten is signed (-1 indicates an error). I don't care; anything other than complete transfer is fatal to the connection. Also, I don't understand how an (in)equality test between signed/unsigned could be dangerous.

I've looked here, here, here, and here, but the questions are all about less-than comparisons, and the answers are all "don't do that".

This question asks about silencing the warning, but a sledgehammer "silence all signed/unsigned" comparisons is undesirable.

How should I silence just this warning in the least intrusive manner possible?

jwm
  • 1,504
  • 1
  • 14
  • 29
  • 1
    `static_cast(toWrite)` – Richard Critten Jun 03 '19 at 17:36
  • _"How should I silence **just this warning** in the least intrusive manner possible?"_ Use an explicit cast at the places you're sure what you're doing. – πάντα ῥεῖ Jun 03 '19 at 17:36
  • The problem goes beyond just "cast it" because he has to be able to detect the explicit failure condition of `::write`, and you have to insure that the cast does not lose useful information so you have to cast into the large positive range. – dmckee --- ex-moderator kitten Jun 03 '19 at 17:43
  • I don't see why you shouldn't change `auto const toWrite = ...` to `ssize_t const toWrite = ...` – Paul Sanders Jun 03 '19 at 20:12
  • @PaulSanders because the system call `::write()` requires an unsigned argument. – jwm Jun 03 '19 at 20:14
  • `write()` doesn't care if you pass it a signed argument, see: https://wandbox.org/permlink/yVy4BCnDkyjscOoM – Paul Sanders Jun 03 '19 at 22:44
  • @PaulSanders. Not quite. By defining `toWrite` as a signed quantity, you basicially cut in half the number of bytes that could be written. Agreed, typically, not a problem. However, if you pass a negative number to a function that takes an unsigned quantity, the compiler will blithely interpret it as a very large unsigned number. That it just happens to work doesn't make it right. – jwm Jun 03 '19 at 22:57
  • Do you really ever want to write more bytes than a `ssize_t` can represent? Honestly? – Paul Sanders Jun 03 '19 at 23:24
  • I guess the point is, there is a difference between me knowing that the conversion is safe, and the compiler knowing that it is safe. You are telling me to rely on the implicit conversion in the function call so that I don't do one in the inequality test. Other than the pragmatics of the situation, I'm having a hard time seeing one as superior to the other. – jwm Jun 04 '19 at 01:04

2 Answers2

4

Separate the detection of the error condition from the detection of a incorrect length and use an explicit cast

if ( nWritten < 0 ||
     static_cast<decltype(toWrite)>(nWritten) != toWrite )
{
   // handle problems
}

Small edit: capture all negative values as errors for a wee bit of futureproofing.

dmckee --- ex-moderator kitten
  • 98,632
  • 24
  • 142
  • 234
  • Thanks for posting an answer. Is the explicit test for -1 necessary? Won't it naturally fall out of the `!=` test? I've been trying to think of pathologies where it might be true, but the only edge I see isn't worth worrying about. – jwm Jun 03 '19 at 17:45
  • @jwm even if that would be the case, why do you want to obfuscate the code? If the two conditions can be conflated into one, then let the compiler do that, it has to know for sure – 463035818_is_not_an_ai Jun 03 '19 at 17:46
  • @jwm What happens when you cast a negative signed value into a unsigned type? But casting the other way also has problems because what happens when you cast from a part of the unsigned range that can't be represented in the signed range? Both of those questions have answers but they are complicated ones; this way lets the reader see exactly what you are doing without having to recall obscure details about how the casts work. – dmckee --- ex-moderator kitten Jun 03 '19 at 17:48
  • @formerlyknownas_463035818 is it obfuscating? What I think the original code says is "if the number of bytes written is not what I expected, fail". What this code says is "if the write returned and error condition, or didn't write as many bytes as expected, fail". It adds an additional condition that is (I believe) subsumed in `!=` – jwm Jun 03 '19 at 17:49
  • @jwm Imagine that you both types are one byte in size and that you wrote 255_10 bytes. On a 2's-complement machine the unsigned representation of 255 and the signed representation of -1 are both 11111111_2. Then what? – dmckee --- ex-moderator kitten Jun 03 '19 at 17:51
  • @jwm You cannot check for -1 and cast to unsigned at the same time, so I would consider it as more clear to keep them seperated and explicit in the code. Even if now you would know that things go well, the -1 check would be implicit in the code and might be missed on future refactorings. And then there is the problem of ambiguity that dmckee is explaining. – 463035818_is_not_an_ai Jun 03 '19 at 17:53
  • I don't want to quibble with you guys; I do appreciate the thought and answer provided. However, I must point out that `size_t` and `ssize_t` are defined as the same width, so if the number of bytes to write is equal to the representation of `static_cast( size_t(-1) )` it will be impossible to distinguish a successful write from an error. So I recognize the edge, and I don't think it can (reasonably) apply. – jwm Jun 03 '19 at 17:58
  • @jwm: `ssize_t` can't represent `size_t(-1)`. The widths may be the same but the signedness differs so the range does too. Furthermore the compiler knows this cannot be represented and may do all sorts of [weird sh!t](https://stackoverflow.com/q/36626810/560648) (or [not so weird](https://stackoverflow.com/a/32133512/560648)) due to the overflow – Lightness Races in Orbit Jun 03 '19 at 18:01
  • @jwm consider that anybody reading the code will have to go through the same process as you did to ensure themself that the code is doing the right thing, when writing the conditions seperately would make this unnecessay, imho that is obfuscation. Anyhow, you cannot concinve me that you can represent `-1` with an unsigned without running into trouble :P – 463035818_is_not_an_ai Jun 03 '19 at 18:02
  • @formerlyknownas_463035818 Converting -1 to unsigned is well-defined due to the modulo arithmetic – Lightness Races in Orbit Jun 03 '19 at 18:03
  • taking @dmckee example, assuming a one byte (s)size_t, assuming I write 255 bytes. Can I detect success or failure? That's why I think it's an edge not worth worrying about. Peace, out. – jwm Jun 03 '19 at 18:06
1

If you can bare some template boilerplate, another possible solution is to write a function which treats each type in a different way:

#include <type_traits>

template <class A, class B>
constexpr bool are_different(A a, B b)
{
    if constexpr (std::is_signed_v<A> and std::is_unsigned_v<B>)
    {
        if ( a < 0 )
            return true;
        else
            return std::make_unsigned_t<A>(a) != b;
    }
    else if constexpr (std::is_unsigned_v<A> and std::is_signed_v<B>)
    {
        if ( b < 0 )
            return true;
        else
            return a != std::make_unsigned_t<B>(b);
    }
    else
    {
        return a != b;
    }
}

int main()
{
    static_assert(are_different(1, 2));
    static_assert(!are_different(1ull, 1));
    static_assert(are_different(1, 2));
    static_assert(are_different(1u, 2));
    static_assert(are_different(1, 2u));
    static_assert(are_different(-1, -1u));
    static_assert(!are_different((long long)-1u, -1u));
}
Bob__
  • 12,361
  • 3
  • 28
  • 42
  • This is interesting. I wasn't aware of `std::make_(un)signed`. I can't find a lot of information on when you might want to use it (I suspect it has utility in certain template-writing applications), but how is it different from `static_cast`? – jwm Jun 03 '19 at 18:44
  • @jwm See e.g. https://en.cppreference.com/w/cpp/types/make_unsigned it *"provides the member typedef type which is the unsigned integer type corresponding to T, with the same cv-qualifiers"* also, with that name, makes your intent clear. – Bob__ Jun 03 '19 at 19:01