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

Submitter:  Jonathan Larmour <jifl>
Submitted:  Wed 11 Apr 2007 03:17:35 AM UTC
   
 
Category:  None Severity:  2 - Minor
Item Group:  Change Request Status:  Fixed
Privacy:  Public Assigned to:  jifl
Open/Closed:  Closed Planned 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>
Group Member
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>
Group Member
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>
Group Member
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>
Group 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>
Group Member

 

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

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 kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jifl (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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-04-11 jifl StatusNeed Info Fixed
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code