jung6717 / arduino

Automatically exported from code.google.com/p/arduino
0 stars 0 forks source link

Patch for Ethernet Library Client bugs in status() and connect() #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Some additional bugs in the original Ethernet Client library made it through
the revisions that were included in 0016. Basically the bugs are sanity
checks in status() and connected() to ensure that a socket is associated
with the client before dispatching a status request to the Wiznet chip. When
a client connection is closed and the socket released, the _sock variable
gets set to 255. Apparently when the invalid socket number 255 is passed to
the Wiznet chip in the getSn_SR(_sock) call, the result (for some unknown
reason) is 1 - which is the status for an open connection! (Sn_IR_CON).

So this results in a nasty situation where a correctly stopped and closed
connection will return true when the connected() function is called and
return an established connection result when status() is called.

Forum reference:
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1238640832/13#13

Below is a patch for Client.cpp (also attached as a file).  Basically it
just adds checks to return a default not connected result if the _sock value
is 255 (not bound to a socket).

--------------------------PATCH BEGIN--------------------------
--- Client.cpp        2009-05-30 14:50:02.000000000 -0400
+++ Client_patched.cpp    2009-06-09 21:05:32.000000000 -0400
@@ -113,13 +113,21 @@
 }

 uint8_t Client::connected() {
-  uint8_t s = status();
-  return !(s == SOCK_LISTEN || s == SOCK_CLOSED || s == SOCK_FIN_WAIT ||
-    (s == SOCK_CLOSE_WAIT && !available()));
+  if (_sock == 255) {
+    return 0;
+  } else {
+    uint8_t s = status();
+    return !(s == SOCK_LISTEN || s == SOCK_CLOSED || s == SOCK_FIN_WAIT ||
+      (s == SOCK_CLOSE_WAIT && !available()));
+  }
 }

 uint8_t Client::status() {
-  return getSn_SR(_sock);
+  if (_sock == 255) {
+    return SOCK_CLOSED;
+  } else {
+    return getSn_SR(_sock);
+  }
 }

 // the next three functions are a hack so we can compare the client
returned
---------------------------PATCH END---------------------------

Here are the revised functions after patching:

---------------------------------------------------------------
uint8_t Client::connected() {
 if (_sock == 255) {
   return 0;
 } else {
   uint8_t s = status();
   return !(s == SOCK_LISTEN || s == SOCK_CLOSED || s == SOCK_FIN_WAIT ||
     (s == SOCK_CLOSE_WAIT && !available()));
 }
}

uint8_t Client::status() {
 if (_sock == 255) {
   return SOCK_CLOSED;
 } else {
   return getSn_SR(_sock);
 }
}
---------------------------------------------------------------

Technically only the patch to the status() function is needed as it will
correctly make the existing connected() function work. Adding the sanity
check to connected() as well makes the code easier to follow.

Original issue reported on code.google.com by josiah.ritchie on 10 Jun 2009 at 12:55

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by dmel...@gmail.com on 15 Jun 2009 at 8:11