Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/external/bsd/dhcpcd/dist/src



    Date:        Fri, 3 Aug 2018 19:46:01 +0100
    From:        Roy Marples <roy%marples.name@localhost>
    Message-ID:  <da95ebf2-7405-2322-2a9c-7b61e086ee66%marples.name@localhost>

  | Considering both methods work and the result of htons is a uint16_t (but 
  | is this always guaranteed?)

ntohs() (not that it matters) and "always" is a big word, but that is how
it is defined by POSIX, so it should be something that we can rely upon.

  | is this just an over-zealous compiler warning?

Not so much over zealous, as just plain lazy... (given Kamil's most
recent message):

  | conversion to 'long unsigned int' from 'int' may change the sign of the
  | result [-Werror=sign-conversion]
  |   *len = ntohs(p->ip.ip_len) - sizeof(p->ip) - sizeof(p->udp);
  |                              ^

That's not the overflow on subtract that you said before, and for this one
I can see how the cast can help ... but that's just sheer laziness on the part
of the compiler.

The "int" which is there was created by the compiler, it knows (or should know)
that the underlying value is in the rangs [0..65535] and cannot possibly have
its sign changed when it is converted to long unsigned int.

It would be more understandable if the int appeared somewhere earlier, but
here this is all in this one expression, one type promotion on top of another.

Get the idiot compiler fixed, and then remove the cast.   In the meantime,
at least mark it with a comment indicating that the cast should not be needed,
and is there purely to appease gcc.

kre



Home | Main Index | Thread Index | Old Index