tasklwIP - A Lightweight TCP/IP stack - Tasks: task #7143, Clean up opt.h

 
 

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

task #7143: Clean up opt.h

Submitter:  Jared Grubb <jgrubb>
Submitted:  Thu 02 Aug 2007 09:48:22 PM UTC
   
 
Category:  Documentation Should Start On:  Thu 02 Aug 2007 07:00:00 AM UTC
Should be Finished on:  Thu 02 Aug 2007 07:00:00 AM UTC Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  jgrubb Percent Complete:  100%
Open/Closed:  Closed Planned Release:  1.3.0
Effort:  0.00

Jump to the original submission

Wed 29 Aug 2007 09:01:01 AM UTC, comment #10: 

Yes please, then I think you can close this.

Kieran Mansley <kieranm>
Group Member
Fri 17 Aug 2007 06:33:35 PM UTC, comment #9: 

Committed to CVS. (I have synced the changes up with the changes made via Task #7136 (memp_std.h stuff) and the changes made by Marc over the last couple days; also I changed IGMP_STATS to depend on LWIP_IGMP).

Should I do an entry in CHANGELOG for this?

Jared Grubb <jgrubb>
Group Member
Thu 16 Aug 2007 04:41:51 PM UTC, comment #8: 

Here is the proposed version.

  • No changes to any values or flags. (Some reordering, did change a couple default stats values, as we discussed earlier)
  • Removed all the "Default" lines that I had in the first version
  • I have left the PPP hard defines alone for now


Anyone object before I commit?

(file #13673)

Jared Grubb <jgrubb>
Group Member
Fri 03 Aug 2007 03:53:03 PM UTC, comment #7: 


> comments like "Default: 0" seems a little bit "strange" since the default value is just after


I've never used these auto-documentation tools (the whole /** thing), so I was imagining maybe these comments would get parsed out for something. If not, then I'll just strip them.



> Note, there is some others options which are in the module header file, and not in opt.h, to make them less "visible", to avoid that any new users change it without know what he do. You can see that like options for "advanced" (???) users.


I didn't know that! :) It would be nice to document those. I figure opt.h can be THE documentation for the standard options (as opposed to using the wiki) since that file is somewhat volatile, but "advanced" options could be put in the wiki maybe?

Jared Grubb <jgrubb>
Group Member
Fri 03 Aug 2007 03:52:49 PM UTC, comment #6: 


>>Of those options you list, only TCP_REXMIT_DEBUG really appears
>> to be unused. The others all are.
>
>I don't find where DEFAULT_THREAD_PRIO is used (perhaps it's in a
>port, but in this case, should be in the lwipopts.h provided with
>the port).


It's in multiple ports. I think the idea is that since it's an option that exists in many ports, it can be put centrally, along with other user-configurable options.

If someone did want to take it out of opt.h, then it will have to be added to the lwipopts.h for each port, although in saying that, now the only supported ports that use it are the three unix port variants.

Jonathan Larmour <jifl>
Group Member
Fri 03 Aug 2007 08:23:41 AM UTC, comment #5: 


>Of those options you list, only TCP_REXMIT_DEBUG really appears to be unused. The others all are.


I don't find where DEFAULT_THREAD_PRIO is used (perhaps it's in a port, but in this case, should be in the lwipopts.h provided with the port).

>comments in opt.h


comments like "Default: 0" seems a little bit "strange" since the default value is just after. I can understand if you create you lwipopts.h file from a copy/paste of a part of opt.h to get comments, but, if we change a default value in opt.h, you don't have to forget to update all these references. To my point of view, it's not useful, but it's just my point of view (it's just a remark).

>Question: Can I add LWIP_STATS_LARGE to the opt.h? Why is it separate?


See https://savannah.nongnu.org/patch/?5811 for my point of view, and the answer to your question.

Note, there is some others options which are in the module header file, and not in opt.h, to make them less "visible", to avoid that any new users change it without know what he do. You can see that like options for "advanced" (???) users.


Frédéric Bernon <fbernon>
Group Member
Fri 03 Aug 2007 07:09:52 AM UTC, comment #4: 

I say go for it.  It was likely set there in the original implementation/patch for convenience.  It would make more sense in opt.h.

Thomas Taranowski <taranowski>
Group Member
Fri 03 Aug 2007 01:25:26 AM UTC, comment #3: 

Question: Can I add LWIP_STATS_LARGE to the opt.h? Why is it separate?

Any other options that you can tune that are not listed in opt.h?

Jared Grubb <jgrubb>
Group Member
Thu 02 Aug 2007 11:34:06 PM UTC, comment #2: 


> I don't see any PPP-related defines that wouldn't be relevant for tuning by the user. Which ones do you mean?


There's a whole list of them that have no #ifndef..#endif around them, for example NUM_PPP or FSM_DEFTIMEOUT. If they're tunable, then I'll put in #ifndef's, otherwise we should move them to ppp.h. I don't use PPP so I'm not sure what to do with them.

> Of those options you list, only TCP_REXMIT_DEBUG really appears to be unused. The others all are.


Ugh. I must've searched wrong. Thanks.

> Like UDP/TCP_STATS, IPFRAG_STATS could also be conditionalised on (IP_REASSEMBLY || IP_FRAG) (it's used for both).


Good call. I've updated it in my local copy. I'll wait for a few more fixes before I upload another version.

Jared Grubb <jgrubb>
Group Member
Thu 02 Aug 2007 11:05:29 PM UTC, comment #1: 

I don't see any PPP-related defines that wouldn't be relevant for tuning by the user. Which ones do you mean?

Of those options you list, only TCP_REXMIT_DEBUG really appears to be unused. The others all are.

Like UDP/TCP_STATS, IPFRAG_STATS could also be conditionalised on (IP_REASSEMBLY || IP_FRAG) (it's used for both).

Jonathan Larmour <jifl>
Group Member
Thu 02 Aug 2007 09:48:22 PM UTC, original submission:  

I have gone through opt.h and made all the commenting uniform (there were all sorts of styles used!). I had to just pick a style, but we can change it.

I tried to make it conform to the "doc style" but I havent done much of that. Should we put in "@deprecated" and others like that?

Also:

  • Changed the default of UDP_STATS, TCP_STATS to be LWIP_UDP and LWIP_TCP (disable the stats if the modules are off)
  • Should there really be #ifdef PPP_SUPPORT ... #endif around all the PPP options? We don't do that for TCP or UDP options, for example.
  • There are a bunch of static defines for PPP. Shouldnt these be moved to ppp.h?
  • DEFAULT_THREAD_PRIO, TCP_RTO_DEBUG, TCP_REXMIT_DEBUG, SLIP_DEBUG, LWIP_DBG_MIN_LEVEL not used in lwIP. Deprecated?
Jared Grubb <jgrubb>
Group Member

 

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

Attached Files
file #13673:  opt.h added by jgrubb (35KiB - text/plain)
file #13560:  opt.h added by jgrubb (40KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by taranowski (Posted a comment)
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by jgrubb (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-08-29 jgrubb Open/ClosedOpen Closed
    2007-08-29 jgrubb StatusReady For Test Done
    2007-08-17 jgrubb StatusNone Ready For Test
        Percent Complete0% 100%
        Assigned toNone jgrubb
        Planned ReleaseNone 1.3.0
    2007-08-16 jgrubb Attached File- Added opt.h, #13673
    2007-08-02 jgrubb Attached File- Added opt.h, #13560

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code