tasklwIP - A Lightweight TCP/IP stack - Tasks: task #6880, netconn API misses some error...

 
 

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

task #6880: netconn API misses some error forwarding

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Sun 13 May 2007 02:24:38 PM UTC
   
 
Category:  None Should Start On:  Sun 13 May 2007 12:00:00 AM UTC
Should be Finished on:  Sun 13 May 2007 12:00:00 AM UTC Priority:  1 - Later
Status:  Done Privacy:  Public
Assigned to:  None Percent Complete:  100%
Open/Closed:  Closed Planned Release:  None
Effort:  0.00

Jump to the original submission

Fri 01 Jun 2007 05:19:37 PM UTC, comment #7: 

Closing it, but opening another one for reviewing the error handling in the netconn layer.

Simon Goldschmidt <goldsimon>
Group administrator
Sat 26 May 2007 12:42:18 PM UTC, comment #6: 

Simon, I think we can close that. Just a thing about one of the last commits for udp_send/udp_sendto (where you have had an error forwarding), now if a MAC reports a error during the send (by example, no "buffers" available to do a "send", in netif's linkoutput), the socket become "unavailable", because most of "error checking" don't allow to do another send. I post a mail about that on the mailing list, "Comments about low_level_output".

Frédéric Bernon <fbernon>
Group Member
Sat 19 May 2007 04:28:26 PM UTC, comment #5: 

I have check in some error forwardings in api_msg. I hope all are processed, else tell me...

Frédéric Bernon <fbernon>
Group Member
Mon 14 May 2007 10:30:07 AM UTC, comment #4: 

OK, that's really better than the switch code. In do_write, however, I would leave the local variable since I'm not sure whether your code leads to multiple access of msg->conn->err in RAM, while the local variable err will always be kept in a register.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 May 2007 10:08:39 AM UTC, comment #3: 

I join a sample to show what about I thought with some "do_" functions. But I'm not sure it can build like this, because, since my last update from CVS, I've got a crash in tcpip_thread... :(



(file #12766)

Frédéric Bernon <fbernon>
Group Member
Mon 14 May 2007 08:59:27 AM UTC, comment #2: 

I don't prefer one to the other, I think. We only have to make sure we do process all errors somehow. What I'm not sure is whether to check that TCP-functions (connect...) are used only called for tcp pcbs or if we cover that using asserts...

About the switch code: I think compilers can optimize this just like big if statements, and I sometimes think the switch is better to read... Maybe you post a patch of what you think?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 May 2007 08:09:59 AM UTC, comment #1: 

I can check it if you want.

If I look in api_msg.c, most of functions report "local" error with a "msg->conn->err = err;" before the "sys_mbox_post", like in :

do_write
do_close
do_join_leave_group

What do you prefer? Directly set conn->err, or only update it at the end?

In a general way, in "do_" functions, some "case" are done where they do nothing, like in do_close (only "case NETCONN_TCP:" is really usefull). Others, like do_write, set errors like "msg->conn->err = ERR_VAL;". I propose to replace most of TCP specific "switch"  by a simple "if (msg->conn->type==NETCONN_TCP)" (code would be simpler, and a switch can't be optimized by compiler).



Frédéric Bernon <fbernon>
Group Member
Sun 13 May 2007 02:24:38 PM UTC, original submission:  

e.g. do_send() calls udp_send or raw_send and doesn't check the return values (should be mapped to msg->conn->err)

I think there are more items (e.g. calling do_listen on a non-tcp netconn returns no error) but I don't have the time to work on it right now.

Simon Goldschmidt <goldsimon>
Group administrator

 

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

Attached Files
file #12766:  api_msg.c.patch added by fbernon (2KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by goldsimon (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-06-01 goldsimon StatusNone Done
        Percent Complete70% 100%
        Open/ClosedOpen Closed
    2007-05-19 fbernon Percent Complete0% 70%
    2007-05-14 fbernon Attached File- Added api_msg.c.patch, #12766

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code