Skip to content

Commit

Permalink
Reduce memory use of TCP PI backend (closes #621)
Browse files Browse the repository at this point in the history
- allocate exact memory required to store node and service strings
  instead of around 1kb of static memory.
- accept NULL value of service to use default Modbus port number (502)
- unit test updated

The new documentation will be updated in another commit.
  • Loading branch information
stephane committed Aug 17, 2022
1 parent 9b679b7 commit ef3c4bc
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 44 deletions.
7 changes: 2 additions & 5 deletions src/modbus-tcp-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,15 @@ typedef struct _modbus_tcp {
char ip[16];
} modbus_tcp_t;

#define _MODBUS_TCP_PI_NODE_LENGTH 1025
#define _MODBUS_TCP_PI_SERVICE_LENGTH 32

typedef struct _modbus_tcp_pi {
/* Transaction ID */
uint16_t t_id;
/* TCP port */
int port;
/* Node */
char node[_MODBUS_TCP_PI_NODE_LENGTH];
char *node;
/* Service */
char service[_MODBUS_TCP_PI_SERVICE_LENGTH];
char *service;
} modbus_tcp_pi_t;

#endif /* MODBUS_TCP_PRIVATE_H */
68 changes: 32 additions & 36 deletions src/modbus-tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,20 @@ static int _modbus_tcp_select(modbus_t *ctx, fd_set *rset, struct timeval *tv, i
}

static void _modbus_tcp_free(modbus_t *ctx) {
free(ctx->backend_data);
if (ctx->backend_data) {
free(ctx->backend_data);
}
free(ctx);
}

static void _modbus_tcp_pi_free(modbus_t *ctx) {
if (ctx->backend_data) {
modbus_tcp_pi_t *ctx_tcp_pi = ctx->backend_data;
free(ctx_tcp_pi->node);
free(ctx_tcp_pi->service);
free(ctx->backend_data);
}

free(ctx);
}

Expand Down Expand Up @@ -786,7 +799,7 @@ const modbus_backend_t _modbus_tcp_pi_backend = {
_modbus_tcp_close,
_modbus_tcp_flush,
_modbus_tcp_select,
_modbus_tcp_free
_modbus_tcp_pi_free
};

modbus_t* modbus_new_tcp(const char *ip, int port)
Expand Down Expand Up @@ -858,8 +871,6 @@ modbus_t* modbus_new_tcp_pi(const char *node, const char *service)
{
modbus_t *ctx;
modbus_tcp_pi_t *ctx_tcp_pi;
size_t dest_size;
size_t ret_size;

ctx = (modbus_t *)malloc(sizeof(modbus_t));
if (ctx == NULL) {
Expand All @@ -879,47 +890,32 @@ modbus_t* modbus_new_tcp_pi(const char *node, const char *service)
return NULL;
}
ctx_tcp_pi = (modbus_tcp_pi_t *)ctx->backend_data;
ctx_tcp_pi->node = NULL;
ctx_tcp_pi->service = NULL;

if (node == NULL) {
/* The node argument can be empty to indicate any hosts */
ctx_tcp_pi->node[0] = 0;
} else {
dest_size = sizeof(char) * _MODBUS_TCP_PI_NODE_LENGTH;
ret_size = strlcpy(ctx_tcp_pi->node, node, dest_size);
if (ret_size == 0) {
fprintf(stderr, "The node string is empty\n");
modbus_free(ctx);
errno = EINVAL;
return NULL;
}

if (ret_size >= dest_size) {
fprintf(stderr, "The node string has been truncated\n");
modbus_free(ctx);
errno = EINVAL;
return NULL;
}
}

if (service != NULL) {
dest_size = sizeof(char) * _MODBUS_TCP_PI_SERVICE_LENGTH;
ret_size = strlcpy(ctx_tcp_pi->service, service, dest_size);
if (node != NULL) {
ctx_tcp_pi->node = strdup(node);
} else {
/* Empty service is not allowed, error caught below. */
ret_size = 0;
/* The node argument can be empty to indicate any hosts */
ctx_tcp_pi->node = strdup("");
}

if (ret_size == 0) {
fprintf(stderr, "The service string is empty\n");
if (ctx_tcp_pi->node == NULL) {
modbus_free(ctx);
errno = EINVAL;
errno = ENOMEM;
return NULL;
}

if (ret_size >= dest_size) {
fprintf(stderr, "The service string has been truncated\n");
if (service != NULL && service[0] != '\0') {
ctx_tcp_pi->service = strdup(service);
} else {
/* Default Modbus port number */
ctx_tcp_pi->service = strdup("502");
}

if (ctx_tcp_pi->service == NULL) {
modbus_free(ctx);
errno = EINVAL;
errno = ENOMEM;
return NULL;
}

Expand Down
3 changes: 0 additions & 3 deletions tests/unit-test-client.c
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,6 @@ int main(int argc, char *argv[])
ctx = modbus_new_rtu("/dev/dummy", 0, 'A', 0, 0);
ASSERT_TRUE(ctx == NULL && errno == EINVAL, "");

ctx = modbus_new_tcp_pi(NULL, NULL);
ASSERT_TRUE(ctx == NULL && errno == EINVAL, "");

printf("\nALL TESTS PASS WITH SUCCESS.\n");
success = TRUE;

Expand Down

0 comments on commit ef3c4bc

Please sign in to comment.