buglwIP - A Lightweight TCP/IP stack - Bugs: bug #33398, Compiler says 'Pointless...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #33398: Compiler says 'Pointless Comparison'

Submitted by:  Mike Kleshov <kleshov>
Submitted on:  Wed 25 May 2011 09:30:00 AM UTC  
 
Category: TCPSeverity: 2 - Minor
Item Group: Compiler WarningStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

Wed 25 May 2011 10:53:21 PM UTC, comment #3:

Sounds like a compiler bug to me. If the compiler's int is 32 bits, 0xFFFF should be treated as a 32-bit signed integer (0x0000FFFF), and u16_t port would be converted to a 32-bit signed integer for the comparison. If port was in the range 0x0000 to 0xFFFE before the increment it would be less than 0x0000FFFF, so the comparison is not always true.

I suspect the compiler is sign extending the 0xFFFF constant in that expression instead of zero extending it. That would result in a comparison testing whether a variable in the range 0x00000000 to 0x0000FFFF is greater than or equal to -1, which is always true. This is clearly a bug. I suggest reporting it to IAR, as it may crop up in other circumstances.

(port++ > 0xFFFF) is being handled correctly, so the bug appears to be limited to the greater-than-or-equal operator. Hopefully changing the comparison to == will avoid the bug.

Other (correctly implemented) compilers would not warn about the comparison, since it is valid (although the greater-than portion of the comparison is superfluous).

David Empson <dempson>
Wed 25 May 2011 05:31:37 PM UTC, comment #2:

Fixed by using == instead of >=

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 25 May 2011 03:42:40 PM UTC, comment #1:

Don't know what I was thinking when I commited 1.142, it should read (port++ == TCP_LOCAL_PORT_RANGE_END).

Thanks for reporting. Unfortunately, my compiler didn't warn about this :-(

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 25 May 2011 09:30:00 AM UTC, original submission:

Here is the snippet of code:

#ifndef TCP_LOCAL_PORT_RANGE_START
/* From http://www.iana.org/assignments/port-numbers:
"The Dynamic and/or Private Ports are those from 49152 through 65535" */
#define TCP_LOCAL_PORT_RANGE_START 0xc000
#define TCP_LOCAL_PORT_RANGE_END 0xffff
#endif
static u16_t port = TCP_LOCAL_PORT_RANGE_START;

again:
if (port++ >= TCP_LOCAL_PORT_RANGE_END) {
port = TCP_LOCAL_PORT_RANGE_START;
}

The line with the if statement causes a compiler warning: 'pointless integer comparison, the result is always true.' The compiler is IAR for ColdFire. Size of int is 32 bits.
The funny thing is when I change '>=' to '>', it says 'pointless integer comparison, the result is always false.'
It could be a compiler bug. But still, wouldn't it make sense to change it to (port++ == TCP_LOCAL_PORT_RANGE_END), like in UDP code? Presumably, port can never be greater than 0xffff since it's u16_t.

Mike Kleshov <kleshov>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by dempson (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by kleshov (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 25 May 2011 05:31:37 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Wed 25 May 2011 03:42:40 PM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon

    Back to the top


    Powered by Savane 3.1-cleanup1