lwIP - A Lightweight TCP/IP stack - Patches: patch #6699, Fixing a couple of compilation...
You are not allowed to post comments on this tracker with your current authentication level.
patch #6699: Fixing a couple of compilation warnings(Paradigm C++)
Submitter: | Tai-hwa Liang <atliang> | ||
Submitted: | Fri 19 Dec 2008 10:15:23 AM UTC | ||
Category: | None | Priority: | 5 - Normal |
Status: | Done | Privacy: | Public |
Assigned to: | goldsimon | Open/Closed: | Closed |
Planned Release: | None |
Jump to the original submission
Thu 26 Feb 2009 02:40:45 AM UTC, comment #16: |
Tai-hwa Liang <atliang> |
Mon 16 Feb 2009 07:38:47 PM UTC, comment #15: I just converted the length parameters to u32_t, so this should be fixed. |
Simon Goldschmidt <goldsimon> |
Thu 12 Feb 2009 05:58:16 PM UTC, comment #14:
|
Simon Goldschmidt <goldsimon> |
Mon 22 Dec 2008 04:59:42 PM UTC, comment #13:
Now I see it... Still, I think the wording of this warning is a bit vague...
I suppose adding an explicit check for size of int would be more readable than relying on the if statement to reduce to 'always false' on 16-bit platforms. Something like this should work:
It won't if you define roll-your-own types like it's currently done with u8_t, u16_t and so on. |
Mike Kleshov <kleshov> |
Mon 22 Dec 2008 04:43:16 PM UTC, comment #12:
|
Bill Auerbach <billauerbach> |
Mon 22 Dec 2008 04:30:00 PM UTC, comment #11:
Do we need to remove that warning? The warning says "Constant out of range in comparison", and I am struggling to understand what it actually means. I believe gcc would say "comparison is always false due to limited range of data type", which is exactly what we want from this code. It is wrong to silence every warning of every compiler. The cost of silencing a warning is not zero: code readability could suffer, and there is a risk of intruducing new bugs (which almost happened in this case.)
I believe the proper solution would be to use C99 types from stdint.h like uint_fast16_t or equivalent. |
Mike Kleshov <kleshov> |
Mon 22 Dec 2008 03:03:39 PM UTC, comment #10: Wouldn't it remove the warning that started this post? It's also marginally more efficient to leave ints and unsigned ints as ints or unsigned ints and not as u16_t/u32_t s_16t/u_16t because int is the native type and most efficient type for the processor. Why force 16/32 bits when there's no benefit gained from doing so?
|
Bill Auerbach <billauerbach> |
Mon 22 Dec 2008 02:55:09 PM UTC, comment #9:
|
Mike Kleshov <kleshov> |
Mon 22 Dec 2008 02:27:53 PM UTC, comment #8: Why not make w.len unsigned int? It can't be negative. 0xffff could also be changed to 0xffffu or MAX_UINT in this case. I generally find it to be a bad idea to mixed signed and unsigned integers.
|
Bill Auerbach <billauerbach> |
Sat 20 Dec 2008 12:25:55 PM UTC, comment #7: Yeah, I reverted it. Sorry for the mess. The warning is back, though, but that's OK. |
Simon Goldschmidt <goldsimon> |
Sat 20 Dec 2008 11:27:28 AM UTC, comment #6: On closer look, it seems that the original behavior (always false) is desireable:
|
Mike Kleshov <kleshov> |
Sat 20 Dec 2008 10:38:21 AM UTC, comment #5:
|
Simon Goldschmidt <goldsimon> |
Sat 20 Dec 2008 12:33:23 AM UTC, comment #4: Mike is correct, including the fix. The type of the len field in struct api_msg_msg and the write offset in struct netconn should both be changed from int to u32_t.
|
Jonathan Larmour <jifl> |
Fri 19 Dec 2008 08:39:24 PM UTC, comment #3: Hi,
|
Mike Kleshov <kleshov> |
Fri 19 Dec 2008 06:14:33 PM UTC, comment #2: I've fixed some of the errors, though not all: the "Conversion may lose significant digits" warnings in pbuf.c, mem.c and sockets.c:
|
Simon Goldschmidt <goldsimon> |
Fri 19 Dec 2008 11:19:50 AM UTC, comment #1: It appears that about half of those warnings are caused by the fact that sizeof(int)==2 for this particular compiler. The other half is just the compiler being overly enthusiastic emitting warnings.
|
Mike Kleshov <kleshov> |
Fri 19 Dec 2008 10:15:23 AM UTC, original submission:
Tested current CVS build with Paradigm C++ 6.00.045:
|
Tai-hwa Liang <atliang> |
Depends on the following items: None found
Items that depend on this one: None found
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 9 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2009-02-26 | atliang | Attached File | - | Added ip_addr.h.patch, #17535 | |
2009-02-16 | goldsimon | Status | In Progress | Done | |
Open/Closed | Open | Closed | |||
2008-12-20 | jifl | Status | Done | In Progress | |
Open/Closed | Closed | Open | |||
2008-12-19 | goldsimon | Status | None | Done | |
Assigned to | None | goldsimon | |||
Open/Closed | Open | Closed | |||
2008-12-19 | atliang | Attached File | - | Added lwip.patch, #17091 |
Powered by Savane 3.13-3230.
Corresponding source code
Adding yet another patch to fix possible building warnings if users use IN_CLASS[ABCD]_{HOST,NET} directly.
(file #17535)