[lwip-devel] [patch #6699] Fixing a couple of compilation warnings(Paradigm C++)

[Mike Kleshov]

Follow-up Comment #3, patch #6699 (project lwip):

Hi,
I am concerned by this change in api_msg.c:

@@ -955 +955 @@
-  if ((conn->write_msg->msg.w.len - conn->write_offset > 0xffff)) { /*
max_u16_t */
+  if ((conn->write_msg->msg.w.len - conn->write_offset > (int)0xffff)) { /*
max_u16_t */

Short version:
It might silence the warning with this particular compiler but it keeps the
bug in the code in case of sizeof(int)==2.

Long version:
The code should work fine if sizeof(int)==4. Let's consider the case of
sizeof(int)==2.
In the original version of the code, the integer constant 0xffff has the type
unsigned int which is u16_t. The left side of operator < will be converted to
u16_t. The unsigned comparison will be pointless (always false.)
In the changed version of the code, the integer constant 0xffff is converted
to int and becomes -1. The signed comparison will be pointless as well (always
true.)
To fix the bug, we need to change the types of both
conn->write_msg->msg.w.len and conn->write_offset. Obviously, the piece of
code in question assumes that both of them are at least s32_t. At the moment
they are int (s16_t).

Other thoughts:
1) In general, I would oppose changes to the code that only intend to silence
questionable warnings of some particular compiler.
2) Why the double parentheses around the condition in this if statement?

- mike

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?6699>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/