stephane / libmodbus

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

modbus_close() fails after modbus_connect() on a disconnected port #354

Open x-o1d opened 8 years ago

x-o1d commented 8 years ago

libmodbus version

3.1.4

Operating system

Linux version 4.8.0-rc1-ga0cba21

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

Modbus RTU client

Expected behavior

If modbus_connect() is called on a disconnected socket, ctx->s should not be replaced by -1, as it removes the possibility of closing the connection(modbus_close) later.

Actual behavior

modbus_connect() called on a disconnected socket replaces ctx->s with -1 and the socket cannot be closed afterwards and the kernel will therefore not reuse the minor number. If the serial device gets disconnected frequently the kernel will eventually run out of minor numbers.

Steps to reproduce the behavior (commands or source code)

libmodbus output with debug mode enabled

stephane commented 8 years ago

I don't see where you saw this problem. Could you give me the github link (file and line) of this problem in master, please?

x-o1d commented 8 years ago

line 591

1) modbus_connect() -- connects to a connected device 2) reads registers 3) device disconnects 4) read register fails 5) check if connection is available with if(modbus_connect(ctx) != -1) 6) ctx->s is replaced with -1 because the device is now disconnected

now the handle can never be closed because the file descriptor for the earlier connection has been replaced with -1 so the kernel will continue to issue a new minor number until the minor numbers run out

This is not a bug per say. Everything is fine if you call modbus_close() everytime prior calling modbus_connect(). But this requirement is not emphasized in the documentation and the idea i got from the documentation is that you can use modbus_connect() to check for an available connection.

The way i see it, this would be a more intuitive approach,

 int tmpfd = open(ctx_rtu->device, flags);

 if (tmpfd == -1) {

    if (ctx->debug) {

        fprintf(stderr, "ERROR Can't open the device %s (%s)\n",

                ctx_rtu->device, strerror(errno));

    }

    return -1;

}

ctx->s = tmpfd;`

This way even if the device was disconnected at the time of calling modbus_connect, ctx->s would still hold the last file descriptor. And modbus_close() can be called at a later time. As shown in the documentation.