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/
_______________________________________________
lwip-devel mailing list
address@hidden
http://lists.nongnu.org/mailman/listinfo/lwip-devel