Reduce memory use of TCP PI backend (closes #621)

- 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.

Reduce memory use of TCP PI backend (closes #621)
- 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.
Stéphane Raimbault
1 parent 9b679b7c
Showing 3 changed files with 34 additions and 44 deletions
src/modbus-tcp-private.h
src/modbus-tcp.c
tests/unit-test-client.c
@@ -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 */
@@ -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);
 }
  
@@ -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)
@@ -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) {
@@ -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;
     }
  
@@ -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;