stephane / libmodbus

A Modbus library for Linux, Mac OS, FreeBSD and Windows
http://libmodbus.org
GNU Lesser General Public License v2.1
3.44k stars 1.75k forks source link

TCP IPv4 modbus_connect() defective #165

Closed ghost closed 10 years ago

ghost commented 10 years ago

i'm on Win7 and use the libmodbus-5.dll created with MinGW(GCC 4.8.1) from the newest 2013-11-18 libmodbus master branch.

after modbus_new_rtu(), modbus_connect() works normal, but after modbus_new_tcp(), modbus_connect() throws always an error and modbus devices cannot be read.

TCP IPv4 modbus_connect() works normal in libmodbus-5.dll's compiled form libmodbus 3.0.5 and earlier, so i could track the defect down to the function _connect() in src/modbus-tcp.c : within the _connect() function the call of connect() in the line

    rc = connect(sockfd, addr, addrlen);

throws the wsaError WSAEWOULDBLOCK , that according to the

microsoft documentation of the connect() function

is something to be expected after a connect() to a nonblocking socket, because the function returns immediately (= nonblocking). but a bit time is needed to establish the connection.

the documentation also describes the solution: that is to check with the select() function if the socket is writeable. exactly this check is already implemented some lines after the call of connect() within the function connect() in src/modbus-tcp.c by the call of select() with enought time (= the "response timeout" in this case) .

so to solve the defect, it's needed to catch the wsaError WSAEWOULDBLOCK .

i suggest the tested solution: in src/modbus-tcp.c within the _connect() function to replace the line

    if (rc == -1 && WSAGetLastError() == WSAEINPROGRESS) {

with

    int wsaError = 0;
    if ( rc == -1 ) { wsaError = WSAGetLastError(); }
    if ( wsaError == WSAEWOULDBLOCK || wsaError == WSAEINPROGRESS ) {

although it would look simpler just to call WSAGetLastError() twice, i favour to call it once and use the saved result, since i don't know if multiple calls of WSAGetLastError() would reliably give always the same result.

morgoth6 commented 10 years ago

Problem with this part of code reported some time ago in https://github.com/stephane/libmodbus/pull/100#discussion_r5179468 I use libmodbus on Win32 with this change for some time not without any major problems.

ghost commented 10 years ago

hello morgoth6,

do you use the patch from pull request 100# or my patch above ?

... not without any major problems.

did you experience a problem with the patch ?

stephane commented 10 years ago

@morgoth6 did you notice any problems with this way to connect?

@MarjanTomas I think it's better to store the result as you propose