stephane / libmodbus

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

can we remove the listening socket nuke at modbus-tcp.c:644 #333

Closed codersmurf closed 8 years ago

codersmurf commented 8 years ago

libmodbus version

all

Operating system

all

Description of the Modbus network (server, client, links, etc)

server

Expected behavior

should not nuke the socket sent in to the accept

Actual behavior

nukes the socket sent into the accept

Steps to reproduce the behavior (commands or source code)

create a non-blocking listening socket and use it in the accept, the socket is obliterated

libmodbus output with debug mode enabled

the lib doesn't mind so no debug errors

Thanks,

Jake

stephane commented 8 years ago

There is nothing at this line: https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L644

Which version do you use?

codersmurf commented 8 years ago

Sorry about that, it starts on line 663, if the socket accept comes back with a bad socket it nukes the listening socket.

Thanks,

Jake

On 6/2/16, Stéphane Raimbault notifications@github.com wrote:

There is nothing at this line: https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L644

Which version do you use?


You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/stephane/libmodbus/issues/333#issuecomment-223284167

codersmurf commented 8 years ago

Were you ever able to see what I was talking about?

Thanks,

On 6/2/16, Jacob Devore codersmurf@gmail.com wrote:

Sorry about that, it starts on line 663, if the socket accept comes back with a bad socket it nukes the listening socket.

Thanks,

Jake

On 6/2/16, Stéphane Raimbault notifications@github.com wrote:

There is nothing at this line: https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L644

Which version do you use?


You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/stephane/libmodbus/issues/333#issuecomment-223284167

stephane commented 8 years ago

https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L663

If the accept's socket is invalid, the listening socket is closed. Why do you want a different behaviour?

codersmurf commented 8 years ago

So I've put the socket in non-blocking and if there is an error because there is no data the socket gets closed out from underneath me.

Thanks,

Jake

On 7/18/16, Stéphane Raimbault notifications@github.com wrote:

https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L663

If the accept's socket is invalid, the listening socket is closed. Why do you want a different behaviour?


You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/stephane/libmodbus/issues/333#issuecomment-233345328

stephane commented 8 years ago

I see

stephane commented 8 years ago

Could you provide a very simple piece of code which put the socket in non-blocking and listen? This way I'll be able to look at the error code (errno) and write a unit test with that.

mhei commented 8 years ago

When I understand @codersmurf correctly, we do not need a unit test for this, just simply remove close(*s) and *s = -1. In other words, the library is given a library-external acquired resource and when this is not usable and/or not in correct state, then the library should not touch it further. This should be left to the application to deal with. An error within the library is already signaled by return code of the function. Application should check for this. Also the documentation does not state that the parameter is mangled in case of error.

stephane commented 8 years ago

Oh I didn't awake this morning... the socket is created by modbus_tcp_socket() but any valid socket could be given to modbus_tcp_accept() and this socket isn't stored in modbus context so you're right, the resource management belongs to the application.

https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L477

BTW: return -1 was missing in TCP PI code...

codersmurf commented 8 years ago

Did u still want some code that puts the socket in nonblocking?

On Jul 19, 2016 5:16 AM, "Stéphane Raimbault" notifications@github.com wrote:

Oh I didn't awake this morning... the socket is created by modbus_tcp_socket() but any valid socket could be given to modbus_tcp_accept() and this socket isn't stored in modbus context so you're right, the resource management belongs to the application.

https://github.com/stephane/libmodbus/blob/master/src/modbus-tcp.c#L477

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/stephane/libmodbus/issues/333#issuecomment-233589901, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4P9lTAhgy0ndYdK8b1cbZ7wDynVnZjks5qXKP-gaJpZM4Ir_P9 .