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'

Submitter:  Mike Kleshov <kleshov>
Submitted:  Wed 25 May 2011 09:30:00 AM UTC
   
 
Category:  TCP Severity:  2 - Minor
Item Group:  Compiler Warning Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned 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>
Group administrator
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>
Group administrator
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>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-05-25 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2011-05-25 goldsimon StatusNone In Progress
        Assigned toNone goldsimon

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code