buglwIP - A Lightweight TCP/IP stack - Bugs: bug #19573, netconn_delete treats 1ms wait as...

 
 

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

bug #19573: netconn_delete treats 1ms wait as poll

Submitted by:  Jonathan Larmour <jifl>
Submitted on:  Wed 11 Apr 2007 03:17:35 AM UTC  
 
Category: NoneSeverity: 2 - Minor
Item Group: Change RequestStatus: Fixed
Privacy: PublicAssigned to: Jonathan Larmour <jifl>
Open/Closed: ClosedPlanned Release: None
lwIP version: None

Wed 11 Apr 2007 03:48:25 PM UTC, comment #4:

Checked in a patch to fix this in line with discussion.

Jonathan Larmour <jifl>
Project MemberIn charge of this item.
Wed 11 Apr 2007 03:08:57 PM UTC, comment #3:

This seems like a sensible change, and I'd rather the API change be visible as this will result in updated ports rather than people continuing to use the old bad way.

Kieran Mansley <kieranm>
Project Administrator
Wed 11 Apr 2007 02:56:52 PM UTC, comment #2:

I'm okay with breaking the ports - if we're going to break them anyway, let's break them all at once :-). But I'll wait for Kieran to say if that's ok in that case, since that sort of thing should not be done lightly.

I guess one alternative choice would not be to have a different function, but to pass a "magic" timeout argument to the current one.

e.g.
#define SYS_MBOX_NOWAIT 0xfffffffe
used as:
sys_arch_mbox_fetch( mbox, msg, SYS_MBOX_NOWAIT );

I don't know if this would really save code in practice, since it still needs to be handled specially anyway. It hides the API change, which is probably a bad thing.

Jonathan Larmour <jifl>
Project MemberIn charge of this item.
Wed 11 Apr 2007 06:49:41 AM UTC, comment #1:

In fact, I had about the same plan. Though I don't know if we really need the define stuff. As you've said earlier, we have broken old ports, anyway.

Not having this function is an error and leads to communication errors, as we can see. And if you solve this with your define, you still must insist on the implementation detail that 1ms wait means 'poll' or else you will get errors, again!

I would be all for implementing this function as a must.

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 11 Apr 2007 03:17:35 AM UTC, original submission:

I raised an issue in bug #19161 that various places in api_lib.c used 1ms waits as if they were polls. In that bug I said I was wrong, but actually I wasn't: netconn_delete does this twice, while trying to purge the recvmbox and acceptmbox.

So I think we do need some form of new sys abstraction to allow a polled wait on an mbox that returns right away if it is not available.

I suggest something along the lines of this in sys.h:

/* sys_mbox_tryfetch() returns SYS_MBOX_EMPTY if appropriate */
#define SYS_MBOX_EMPTY SYS_ARCH_TIMEOUT

#if SYS_HAS_MBOX_TRYFETCH
extern u32_t sys_arch_mbox_tryfetch(sys_mbox_t mbox, void **msg);
#define sys_mbox_tryfetch(_mb,_m) \
sys_arch_mbox_tryfetch((_mb),(_m))
#else
#define sys_mbox_tryfetch(_mb,_m) \
sys_arch_mbox_fetch((_mb),(_m), 1)
#endif

with a corresponding #define in opt.h for SYS_HAS_MBOX_TRYFETCH. Initially this will default to 0 to preserve existing behaviour for ports. Perhaps in a while it can default to 1.

But I know there's a lot going on in this area, so I want to know if anyone had any other plans for this function before I do something.

Jonathan Larmour <jifl>
Project MemberIn charge of this item.

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by jifl (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 11 Apr 2007 03:48:25 PM UTCjiflStatusNeed Info=>Fixed
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup1