leo25 / arduino

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

recv() can return -1 in an unsigned int (socket.cpp in the Ethernet library). #516

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The recv() function in the Ethernet library (socket.cpp) can return -1 (to 
indicate a lack of available data) but its return type is unsigned (uint16_t).  
This seems like it could cause problems - or, at least, it feels that way 
reading the code.  Can you check what happens in this case?

Original issue reported on code.google.com by dmel...@gmail.com on 29 Mar 2011 at 2:01

GoogleCodeExporter commented 9 years ago
I have issues related to this one. Please see

http://arduino.cc/forum/index.php/topic,57972.0.html

Maybe you want to take them into account.
(It relates to arduino sw version 22)

Original comment by ts...@gmx.net on 14 Apr 2011 at 2:45

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
The same goes for recvfrom() which also have trouble with a potential overflow. 
Discussed in issue #669

Original comment by pe...@birchroad.net on 7 Dec 2011 at 10:54

GoogleCodeExporter commented 9 years ago
From Shawn Wallace:

"We've been testing the recipes in the Arduino Cookbook (O'Reilly) and found a 
bug in the Ethernet Client. Attached is a sketch that fails with the 1.0 
release of the Ethernet library. It works with the rc1 release; it turns out 
that the fix for issue 640 revealed another bug. Issue 640 fixed this line in 
the socket.cpp file:

 if ( s == SnSR::LISTEN || s == SnSR::CLOSED || s == SnSR::CLOSE_WAIT )

to

 if ( status == SnSR::LISTEN || status == SnSR::CLOSED || status == SnSR::CLOSE_WAIT )

which checks the status instead of the socket index, which is correct. However, 
the previous line always tested true, shortcircuiting the test so that recv 
never returned -1. The revealed bug is that the return value for recv should be 
a signed integer, not an uint16_t. So here's a fix:

In EthernetClient/utility/socket.cpp:

146c146
< uint16_t recv(SOCKET s, uint8_t *buf, uint16_t len)
---
int16_t recv(SOCKET s, uint8_t *buf, uint16_t len)
149c149
<   uint16_t ret = W5100.getRXReceivedSize(s);
---
 int16_t ret = W5100.getRXReceivedSize(s);
153a154

In EthernetClient/utility/socket.h:

12c12
< extern uint16_t recv(SOCKET s, uint8_t * buf, uint16_t len);  // Receive data 
(TCP)
---
extern int16_t recv(SOCKET s, uint8_t * buf, uint16_t len); // Receive data 
(TCP)
"

Original comment by dmel...@gmail.com on 8 Dec 2011 at 10:28

Attachments:

GoogleCodeExporter commented 9 years ago
https://github.com/arduino/Arduino/commit/7c90d9d8b55962bd4f5fc9d0fd40432f269f78
77

Original comment by dmel...@gmail.com on 8 Dec 2011 at 10:32