[Top][All Lists]

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

[lwip-devel] [patch #6699] Fixing a couple of compilation warnings(Parad

From: Mike Kleshov
Subject: [lwip-devel] [patch #6699] Fixing a couple of compilation warnings(Paradigm C++)
Date: Fri, 19 Dec 2008 20:39:25 +0000
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv: Gecko/2008102920 Firefox/3.0.4

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

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
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
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:


  Message sent via/by Savannah

reply via email to

[Prev in Thread] Current Thread [Next in Thread]