lwIP - A Lightweight TCP/IP stack - Patches: patch #5865, Fix missing lwip directory in...
You are not allowed to post comments on this tracker with your current authentication level.
patch #5865: Fix missing lwip directory in socket.h including path
Submitter: | Tai-hwa Liang <atliang> | ||
Submitted: | Thu 12 Apr 2007 08:50:20 AM UTC | ||
Category: | None | Priority: | 3 - Low |
Status: | Done | Privacy: | Public |
Assigned to: | kieranm | Open/Closed: | Closed |
Planned Release: | None |
Jump to the original submission
Mon 21 May 2007 04:33:18 PM UTC, comment #19: |
Frédéric Bernon <fbernon> |
Mon 14 May 2007 12:03:32 PM UTC, comment #18: Frédéric has checked that in, already.
|
Simon Goldschmidt <goldsimon> |
Mon 14 May 2007 10:47:15 AM UTC, comment #17: I'm with Frederic that it should be included. sockets.h is effectively the API file, and it should be defining htonl etc.
|
Jonathan Larmour <jifl> |
Mon 14 May 2007 08:26:33 AM UTC, comment #16: about include inet.h, I'm agree it's not directly use by sockets functions (but this is the same thing for ip_addr.h). In most of application ports I do, only sockets.h was necessary. From memory, sockets.h is used to make a "more" portable code (it is defined in most of ip stacks), because, even in unix, the real file to call is not sockets.h, but socket.h (exactly, sys/socket.h, see http://www.opengroup.org/onlinepubs/007908799/xns/syssocket.h.html). Most of time, one include the other one.
|
Frédéric Bernon <fbernon> |
Mon 14 May 2007 07:56:49 AM UTC, comment #15: About the hton-warnings: I know the warnings appear now, and we could get rid of them by including inet.h in sockets.h. But strictly, sockets.h does not need the hton-functions and maybe the user wants to use others than the lwIP-internal ones. Coding on linux, you also have to include inet.h (or how is it called? I can't remember...) if you want to use hton-functions...
|
Simon Goldschmidt <goldsimon> |
Mon 14 May 2007 07:48:04 AM UTC, comment #14: Hmm, seems with removing tcp.h from sockets.h, I got now lot of warnings like :
|
Frédéric Bernon <fbernon> |
Sun 13 May 2007 02:34:25 PM UTC, comment #13:
|
Simon Goldschmidt <goldsimon> |
Sun 13 May 2007 02:19:30 PM UTC, comment #12: Yeah, I think I must have read LWIP_TCP_KEEPALIVE as TCP_KEEPALIVE in tcp.c. In that case I do still agree with Simon after all ;). The patch appears sane - go for it as far as I'm concerned.
|
Jonathan Larmour <jifl> |
Sun 13 May 2007 02:06:14 PM UTC, comment #11:
|
Simon Goldschmidt <goldsimon> |
Fri 20 Apr 2007 01:59:06 PM UTC, comment #10: While I initially agreed with Simon, I'm not sure now, given that I notice that both TCP_KEEPALIVE and TCP_MAXIDLE are used in the lwIP core code. Given that, they should be in a core header. And I think the options should live in the same headerfile together. Some different defines could separate the abstractions without penalty.
|
Jonathan Larmour <jifl> |
Fri 20 Apr 2007 08:35:20 AM UTC, comment #9: To protect the point of view for the current implementation: linux does it the same way (defining TCP_NODELAY etc. in tcp.h and thus exporting all tcp structs with socket.h), but I still think it would be better to separate sockets (and their options) from internal implementation. |
Simon Goldschmidt <goldsimon> |
Fri 20 Apr 2007 08:00:58 AM UTC, comment #8: I have to say I agree with Simon on this. I'll investigate how these options could be moved around to prevent the need for this dependency |
Kieran Mansley <kieranm> |
Fri 20 Apr 2007 06:29:07 AM UTC, comment #7: I just think it's strange that the Options for level IPPROTO_IP are included in sockets.h while the tcp options are in tcp.h, although they have nothing to do with our tcp implementation but are a arguments to a function in sockets.c |
Simon Goldschmidt <goldsimon> |
Fri 20 Apr 2007 06:20:39 AM UTC, comment #6: Hi,
|
Frédéric Bernon <fbernon> |
Fri 20 Apr 2007 02:07:20 AM UTC, comment #5: I guess that one of the reason to put TCP_NODELAY inside tcp.h is to keep all user-settable options together, as commented in lwip/tcp.h:
*/
|
Tai-hwa Liang <atliang> |
Thu 19 Apr 2007 06:27:42 PM UTC, comment #4: Good point... but I still think it would be better not to include the raw-API function definitions in socket.h to prevent users from accessing something directly...
|
Simon Goldschmidt <goldsimon> |
Thu 19 Apr 2007 06:13:57 PM UTC, comment #3: From the CVS log:
|
Jonathan Larmour <jifl> |
Thu 19 Apr 2007 06:07:40 PM UTC, comment #2: Before applying this patch, why is tcp.h included in sockets.h anyway? For me, it compiles without and I think it is better to remove it. |
Simon Goldschmidt <goldsimon> |
Thu 12 Apr 2007 09:50:46 AM UTC, comment #1: Looks good to me. I'll apply when I next get the chance |
Kieran Mansley <kieranm> |
Thu 12 Apr 2007 08:50:20 AM UTC, original submission:
The attached patch tries to add a "lwip/" before local including headers as we already did in the rest of header/implementation files.
|
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 5 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2007-05-22 | kieranm | Status | None | Done | |
Open/Closed | Open | Closed | |||
2007-05-13 | goldsimon | Attached File | - | Added sockets_h_include.patch, #12763 | |
2007-04-12 | kieranm | Assigned to | None | kieranm | |
2007-04-12 | atliang | Attached File | - | Added socket.h.patch, #12471 |
Powered by Savane 3.13-d3ae.
Corresponding source code
Kieran, I think you can close it...