patchlwIP - A Lightweight TCP/IP stack - Patches: patch #6480, slipif.c has no #ifdef

 
 

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

patch #6480: slipif.c has no #ifdef

Submitted by:  Iordan Neshev <iordan_neshev>
Submitted on:  Wed 26 Mar 2008 02:46:35 PM UTC  
 
Category:  None Priority:  3 - Low
Status:  Done Privacy:  Public
Assigned to:  Frédéric Bernon <fbernon> Open/Closed:  Closed
Planned Release:  None

( Jump to the original submission)

Tue 17 Jun 2008 08:14:37 PM UTC, comment #9: 

I've just checked this in now.

Simon Goldschmidt <goldsimon>
Project Administrator
Mon 05 May 2008 05:05:14 AM UTC, comment #8: 

Hmm, I must say I don't have a strong opinion on this any more... Go ahead and change it if you like.

Simon Goldschmidt <goldsimon>
Project Administrator
Sun 04 May 2008 07:49:39 PM UTC, comment #7: 

Simon, are you always against that ? Or do you prefer to close it as invalid ?

Frédéric Bernon <fbernon>
Project MemberIn charge of this item.
Sun 30 Mar 2008 07:54:28 AM UTC, comment #6: 

>LWIP_HAVE_LOOPIF is used by dns.c.

yes, but just since december, and it was in opt.h since several months...

Don't include it in opt.h would be (and is) an exception, since all others lwip "modules" files can be included in a makefile without any problems (and you just choose what you want with you lwipopts.h).

Frédéric Bernon <fbernon>
Project MemberIn charge of this item.
Thu 27 Mar 2008 07:13:51 PM UTC, comment #5: 

LWIP_HAVE_LOOPIF is used by dns.c.
PPP_SUPPORT is used in init.c, admittedly only for a sanity check.

Jonathan Larmour <jifl>
Project Member
Thu 27 Mar 2008 06:38:50 PM UTC, comment #4: 

About comment #2, in this case, why have LWIP_HAVE_LOOPIF and PPP_SUPPORT in opt.h? Since slipif is one of the default netif provided by lwIP, it should be logical to do the same for these others modules.

Frédéric Bernon <fbernon>
Project MemberIn charge of this item.
Thu 27 Mar 2008 06:34:36 PM UTC, comment #3: 

You are right. opt.h is large enough.
It's better to exclude slipif.c from the build.
By the way, I don't use slipif, just noticed
that it is a bit different and decided to post
it here.

Iordan Neshev <iordan_neshev>
Thu 27 Mar 2008 06:17:30 PM UTC, comment #2: 

I'd strongly vote against this: just compile slipif.c only if you want it to go into the build. Including the LWIP_HAVE_SLIPIF=1 in opt.h would make the file bigger without helping much: not many people will want this enabled...

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 26 Mar 2008 06:33:52 PM UTC, comment #1: 

No really necessary, but in the spirit of the others netifs.

Even if this is something which cause a port breakage (need to set LWIP_HAVE_SLIPIF=1 in lwipopts.h), this is something we can do...

Frédéric Bernon <fbernon>
Project MemberIn charge of this item.
Wed 26 Mar 2008 02:46:35 PM UTC, original submission:  

Hello

I think that slipif.c should begin with:

#include "netif/slipif.h"
#include "lwip/opt.h"

#if LWIP_HAVE_SLIPIF    <--- this is missing

#include "lwip/def.h"

....

and end with
#endif /* LWIP_HAVE_SLIPIF */   <-- missing

Of course, this should be added in opt.h:

#ifndef LWIP_HAVE_SLIPIF
#define LWIP_HAVE_SLIPIF 0
#endif

Iordan Neshev <iordan_neshev>

 

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 jifl (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by iordan_neshev (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-06-17 goldsimon Open/ClosedOpen => Closed
    2008-06-17 goldsimon StatusNone => Done
    2008-03-26 fbernon Priority5 - Normal => 3 - Low
        Assigned toNone => fbernon

    Back to the top


    Powered by Savane 3.5