Check the number of points requested

- s/datas/data/
- the warning is now an error
- add some checks in modbus.c
- add unit tests
- remove the TODO item
- minor 80 columns changes

TODO

@@ -1,5 +1,4 @@
 Features
-* check the count, eg. read_holding_registers (MAX_STATUS, etc)
 * broadcasting
 * slave must listen only request sent for him
 

modbus/modbus.c

@@ -533,8 +533,8 @@ int receive_msg(modbus_param_t *mb_param,
                 /* Sums bytes received */ 
                 (*msg_length) += read_ret;
                 if ((*msg_length) > MAX_MESSAGE_LENGTH) {
-                        error_treat(mb_param, TOO_MANY_DATAS, "Too many datas");
-                        return TOO_MANY_DATAS;
+                        error_treat(mb_param, TOO_MANY_DATA, "Too many data");
+                        return TOO_MANY_DATA;
                 }
 
                 /* Display the hex code of each character received */
@@ -566,7 +566,7 @@ int receive_msg(modbus_param_t *mb_param,
                         }
                 }
 
-                /* Moves the pointer to receive other datas */
+                /* Moves the pointer to receive other data */
                 p_msg = &(p_msg[read_ret]);
 
                 if (length_to_read > 0) {
@@ -969,10 +969,16 @@ static int read_io_status(modbus_param_t *mb_param, int slave, int function,
 /* Reads the boolean status of coils and sets the array elements
    in the destination to TRUE or FALSE. */
 int read_coil_status(modbus_param_t *mb_param, int slave, int start_addr,
-                     const int count, uint8_t *data_dest)
+                     int count, uint8_t *data_dest)
 {
         int status;
 
+        if (count > MAX_STATUS) {
+                printf("ERROR Too many coils status requested (%d > %d)\n",
+                       count, MAX_STATUS);
+                return TOO_MANY_DATA;
+        }
+
         status = read_io_status(mb_param, slave, FC_READ_COIL_STATUS,
                                 start_addr, count, data_dest);
 
@@ -985,10 +991,16 @@ int read_coil_status(modbus_param_t *mb_param, int slave, int start_addr,
 
 /* Same as read_coil_status but reads the slaves input table */
 int read_input_status(modbus_param_t *mb_param, int slave, int start_addr,
-                      const int count, uint8_t *data_dest)
+                      int count, uint8_t *data_dest)
 {
         int status;
 
+        if (count > MAX_STATUS) {
+                printf("ERROR Too many inputs status requested (%d > %d)\n",
+                       count, MAX_STATUS);
+                return TOO_MANY_DATA;
+        }
+
         status = read_io_status(mb_param, slave, FC_READ_INPUT_STATUS,
                                 start_addr, count, data_dest);
 
@@ -1007,6 +1019,12 @@ static int read_registers(modbus_param_t *mb_param, int slave, int function,
         int query_ret;
         uint8_t query[MIN_QUERY_LENGTH];
 
+        if (count > MAX_REGISTERS) {
+                printf("EROOR Too many holding registers requested (%d > %d)\n",
+                       count, MAX_REGISTERS);
+                return TOO_MANY_DATA;
+        }
+
         query_length = build_query_basis(mb_param, slave, function, 
                                          start_addr, count, query);
 
@@ -1027,9 +1045,9 @@ int read_holding_registers(modbus_param_t *mb_param, int slave,
         int status;
 
         if (count > MAX_REGISTERS) {
-                printf("WARNING Too many holding registers requested (%d > %d)\n",
+                printf("ERROR Too many holding registers requested (%d > %d)\n",
                        count, MAX_REGISTERS);
-                count = MAX_REGISTERS;
+                return TOO_MANY_DATA;
         }
 
         status = read_registers(mb_param, slave, FC_READ_HOLDING_REGISTERS,
@@ -1045,9 +1063,9 @@ int read_input_registers(modbus_param_t *mb_param, int slave,
         int status;
 
         if (count > MAX_REGISTERS) {
-                printf("WARNING Too many input registers requested (%d > %d)\n",
+                printf("ERROR Too many input registers requested (%d > %d)\n",
                        count, MAX_REGISTERS);
-                count = MAX_REGISTERS;
+                return TOO_MANY_DATA;
         }
 
         status = read_registers(mb_param, slave, FC_READ_INPUT_REGISTERS,
@@ -1156,9 +1174,9 @@ int force_multiple_coils(modbus_param_t *mb_param, int slave,
         uint8_t query[MAX_MESSAGE_LENGTH];
 
         if (nb_points > MAX_STATUS) {
-                printf("WARNING Writing to too many coils (%d > %d)\n",
+                printf("ERROR Writing to too many coils (%d > %d)\n",
                        nb_points, MAX_STATUS);
-                nb_points = MAX_STATUS;
+                return TOO_MANY_DATA;
         }
 
         query_length = build_query_basis(mb_param, slave,
@@ -1206,9 +1224,9 @@ int preset_multiple_registers(modbus_param_t *mb_param, int slave,
         uint8_t query[MAX_MESSAGE_LENGTH];
 
         if (nb_points > MAX_REGISTERS) {
-                printf("WARNING Trying to write to too many registers (%d > %d)\n",
+                printf("ERROR Trying to write to too many registers (%d > %d)\n",
                        nb_points, MAX_REGISTERS);
-                nb_points = MAX_REGISTERS;
+                return TOO_MANY_DATA;
         }
 
         query_length = build_query_basis(mb_param, slave,

modbus/modbus.h

@@ -99,7 +99,7 @@ extern "C" {
 #define COMM_TIME_OUT           -0x0C
 #define PORT_SOCKET_FAILURE     -0x0D
 #define SELECT_FAILURE          -0x0E
-#define TOO_MANY_DATAS          -0x0F
+#define TOO_MANY_DATA           -0x0F
 #define INVALID_CRC             -0x10
 #define INVALID_EXCEPTION_CODE  -0x11
 #define CONNECTION_CLOSED       -0x12

tests/unit-test-master.c

@@ -54,14 +54,15 @@ int main(void)
         memset(tab_rp_status, 0, nb_points * sizeof(uint8_t));
         
         /* Allocate and initialize the memory to store the registers */
-        nb_points = (UT_HOLDING_REGISTERS_NB_POINTS > UT_INPUT_REGISTERS_NB_POINTS) ?
+        nb_points = (UT_HOLDING_REGISTERS_NB_POINTS > 
+                     UT_INPUT_REGISTERS_NB_POINTS) ?
                 UT_HOLDING_REGISTERS_NB_POINTS : UT_INPUT_REGISTERS_NB_POINTS;
         tab_rp_registers = (uint16_t *) malloc(nb_points * sizeof(uint16_t));
         memset(tab_rp_registers, 0, nb_points * sizeof(uint16_t));
 
-        printf("UNIT TESTING\n");
+        printf("** UNIT TESTING **\n");
 
-        printf("Test read functions:\n");
+        printf("TEST READ FUNCTIONS:\n");
 
         /** COIL STATUS **/
         ret = read_coil_status(&mb_param, SLAVE, UT_COIL_STATUS_ADDRESS,
@@ -93,7 +94,7 @@ int main(void)
         /** INPUT STATUS **/
         ret = read_input_status(&mb_param, SLAVE, UT_INPUT_STATUS_ADDRESS,
                                 UT_INPUT_STATUS_NB_POINTS, tab_rp_status);
-        printf("* input status :");
+        printf("* read_input_status: ");
 
         if (ret != UT_INPUT_STATUS_NB_POINTS) {
                 printf("FAILED (nb points %d)\n", ret); 
@@ -119,9 +120,11 @@ int main(void)
         printf("OK\n");
 
         /** HOLDING REGISTERS **/
-        ret = read_holding_registers(&mb_param, SLAVE, UT_HOLDING_REGISTERS_ADDRESS,
-                                     UT_HOLDING_REGISTERS_NB_POINTS, tab_rp_registers);
-        printf("* holding registers: ");
+        ret = read_holding_registers(&mb_param,
+                                     SLAVE, UT_HOLDING_REGISTERS_ADDRESS,
+                                     UT_HOLDING_REGISTERS_NB_POINTS,
+                                     tab_rp_registers);
+        printf("* read_holding_registers: ");
         if (ret != UT_HOLDING_REGISTERS_NB_POINTS) {
                 printf("FAILED (nb points %d)\n", ret); 
                 goto close;
@@ -130,16 +133,19 @@ int main(void)
         for (i=0; i < UT_HOLDING_REGISTERS_NB_POINTS; i++) {
                 if (tab_rp_registers[i] != UT_HOLDING_REGISTERS_TAB[i]) {
                         printf("FAILED (%0X != %0X)\n",
-                               tab_rp_registers[i], UT_HOLDING_REGISTERS_TAB[i]);
+                               tab_rp_registers[i],
+                               UT_HOLDING_REGISTERS_TAB[i]);
                         goto close;
                 }
         }
         printf("OK\n");
 
         /** INPUT REGISTERS **/
-        ret = read_input_registers(&mb_param, SLAVE, UT_INPUT_REGISTERS_ADDRESS,
-                                   UT_INPUT_REGISTERS_NB_POINTS, tab_rp_registers);
-        printf("* input registers: ");
+        ret = read_input_registers(&mb_param,
+                                   SLAVE, UT_INPUT_REGISTERS_ADDRESS,
+                                   UT_INPUT_REGISTERS_NB_POINTS,
+                                   tab_rp_registers);
+        printf("* read_input_registers: ");
         if (ret != UT_INPUT_REGISTERS_NB_POINTS) {
                 printf("FAILED (nb points %d)\n", ret); 
                 goto close;
@@ -155,10 +161,10 @@ int main(void)
         printf("OK\n");
 
         /** WRITE FUNCTIONS **/
-        printf("\nTest write functions:\n");
+        printf("\nTEST WRITE FUNCTIONS:\n");
         
         ret = force_single_coil(&mb_param, SLAVE, UT_COIL_STATUS_ADDRESS, ON);
-        printf("* force single coil: ");
+        printf("* force_single_coil: ");
         if (ret == 1) {
                 printf("OK\n");
         } else {
@@ -177,8 +183,11 @@ int main(void)
 
         {
                 int nb_points = 0x0A;
-                uint8_t tab_value[] = { ON, OFF, ON, ON, OFF, OFF, ON, ON, ON, OFF };
-                ret = force_multiple_coils(&mb_param, SLAVE, 0x13, nb_points, tab_value);
+                uint8_t tab_value[] = { ON, OFF, ON, ON,
+                                        OFF, OFF, ON, ON,
+                                        ON, OFF };
+                ret = force_multiple_coils(&mb_param, SLAVE, 0x13,
+                                           nb_points, tab_value);
                 printf("* force_multiple_coils: ");
                 if (ret == nb_points) {
                         printf("OK\n");
@@ -191,7 +200,8 @@ int main(void)
         {
                 int nb_points = 2;
                 uint16_t tab_value[] = { 0x000A, 0x0102 };
-                ret = preset_multiple_registers(&mb_param, SLAVE, 0x01, nb_points, tab_value);
+                ret = preset_multiple_registers(&mb_param, SLAVE,
+                                                0x01, nb_points, tab_value);
                 printf("* preset_multiple_registers: ");
                 if (ret == nb_points) {
                         printf("OK\n");
@@ -202,46 +212,55 @@ int main(void)
         }
 
         /** ILLEGAL DATA ADDRESS */
-        printf("\nTest illegal data address:\n");
+        printf("\nTEST ILLEGAL DATA ADDRESS:\n");
 
-        /* The mapping begins at 0 and end at adresse + nb_points so
-         * the adresses above are not valid. */ 
+        /* The mapping begins at 0 and ending at address + nb_points so
+         * the addresses below are not valid. */ 
         
-        ret = read_coil_status(&mb_param, SLAVE, UT_COIL_STATUS_ADDRESS,
-                               UT_COIL_STATUS_NB_POINTS + 1, tab_rp_status);
-        printf("* coil status: ");
+        ret = read_coil_status(&mb_param, SLAVE,
+                               UT_COIL_STATUS_ADDRESS,
+                               UT_COIL_STATUS_NB_POINTS + 1,
+                               tab_rp_status);
+        printf("* read_coil_status: ");
         if (ret == ILLEGAL_DATA_ADDRESS)
                 printf("OK\n");
         else
                 printf("FAILED\n");
 
-        ret = read_input_status(&mb_param, SLAVE, UT_INPUT_STATUS_ADDRESS,
-                                UT_INPUT_STATUS_NB_POINTS + 1, tab_rp_status);
-        printf("* input status: ");
+        ret = read_input_status(&mb_param, SLAVE,
+                                UT_INPUT_STATUS_ADDRESS,
+                                UT_INPUT_STATUS_NB_POINTS + 1,
+                                tab_rp_status);
+        printf("* read_input_status: ");
         if (ret == ILLEGAL_DATA_ADDRESS)
                 printf("OK\n");
         else
                 printf("FAILED\n");
 
-        ret = read_holding_registers(&mb_param, SLAVE, UT_HOLDING_REGISTERS_ADDRESS,
-                                     UT_HOLDING_REGISTERS_NB_POINTS + 1, tab_rp_registers);
-        printf("* holding registers: ");
+        ret = read_holding_registers(&mb_param, SLAVE,
+                                     UT_HOLDING_REGISTERS_ADDRESS,
+                                     UT_HOLDING_REGISTERS_NB_POINTS + 1,
+                                     tab_rp_registers);
+        printf("* read_holding_registers: ");
         if (ret == ILLEGAL_DATA_ADDRESS)
                 printf("OK\n");
         else
                 printf("FAILED\n");
                 
-        ret = read_input_registers(&mb_param, SLAVE, UT_INPUT_REGISTERS_ADDRESS,
-                                   UT_INPUT_REGISTERS_NB_POINTS + 1, tab_rp_registers);
-        printf("* input registers: ");
+        ret = read_input_registers(&mb_param, SLAVE,
+                                   UT_INPUT_REGISTERS_ADDRESS,
+                                   UT_INPUT_REGISTERS_NB_POINTS + 1,
+                                   tab_rp_registers);
+        printf("* read_input_registers: ");
         if (ret == ILLEGAL_DATA_ADDRESS)
                 printf("OK\n");
         else
                 printf("FAILED\n");
 
         ret = force_single_coil(&mb_param, SLAVE,
-                                UT_COIL_STATUS_ADDRESS + UT_COIL_STATUS_NB_POINTS, ON);
-        printf("* force single coil: ");
+                                UT_COIL_STATUS_ADDRESS + UT_COIL_STATUS_NB_POINTS,
+                                ON);
+        printf("* force_single_coil: ");
         if (ret == ILLEGAL_DATA_ADDRESS) {
                 printf("OK\n");
         } else {
@@ -250,8 +269,9 @@ int main(void)
 
         ret = force_multiple_coils(&mb_param, SLAVE,
                                    UT_COIL_STATUS_ADDRESS + UT_COIL_STATUS_NB_POINTS,
-                                   UT_COIL_STATUS_NB_POINTS, tab_rp_status);
-        printf("* force multipls coils: ");
+                                   UT_COIL_STATUS_NB_POINTS,
+                                   tab_rp_status);
+        printf("* force_multiple_coils: ");
         if (ret == ILLEGAL_DATA_ADDRESS) {
                 printf("OK\n");
         } else {
@@ -260,14 +280,81 @@ int main(void)
 
         ret = preset_multiple_registers(&mb_param, SLAVE,
                                         UT_HOLDING_REGISTERS_ADDRESS + UT_HOLDING_REGISTERS_NB_POINTS,
-                                        UT_HOLDING_REGISTERS_NB_POINTS, tab_rp_registers);
-        printf("* preset multiple registers: ");
+                                        UT_HOLDING_REGISTERS_NB_POINTS,
+                                        tab_rp_registers);
+        printf("* preset_multiple_registers: ");
         if (ret == ILLEGAL_DATA_ADDRESS) {
                 printf("OK\n");
         } else {
                 printf("FAILED\n");
         }
 
+
+        /** TOO MANY DATA */
+        printf("\nTEST TOO MANY DATA ERROR:\n");
+
+        ret = read_coil_status(&mb_param, SLAVE,
+                               UT_COIL_STATUS_ADDRESS,
+                               MAX_STATUS + 1,
+                               tab_rp_status);
+        printf("* read_coil_status: ");
+        if (ret == TOO_MANY_DATA)
+                printf("OK\n");
+        else
+                printf("FAILED\n");
+
+        ret = read_input_status(&mb_param, SLAVE,
+                                UT_INPUT_STATUS_ADDRESS,
+                                MAX_STATUS + 1,
+                                tab_rp_status);
+        printf("* read_input_status: ");
+        if (ret == TOO_MANY_DATA)
+                printf("OK\n");
+        else
+                printf("FAILED\n");
+
+        ret = read_holding_registers(&mb_param, SLAVE,
+                                     UT_HOLDING_REGISTERS_ADDRESS,
+                                     MAX_REGISTERS + 1,
+                                     tab_rp_registers);
+        printf("* read_holding_registers: ");
+        if (ret == TOO_MANY_DATA)
+                printf("OK\n");
+        else
+                printf("FAILED\n");
+                
+        ret = read_input_registers(&mb_param, SLAVE,
+                                   UT_INPUT_REGISTERS_ADDRESS,
+                                   MAX_REGISTERS + 1,
+                                   tab_rp_registers);
+        printf("* read_input_registers: ");
+        if (ret == TOO_MANY_DATA)
+                printf("OK\n");
+        else
+                printf("FAILED\n");
+
+        ret = force_multiple_coils(&mb_param, SLAVE,
+                                   UT_COIL_STATUS_ADDRESS,
+                                   MAX_STATUS + 1,
+                                   tab_rp_status);
+        printf("* force_multiple_coils: ");
+        if (ret == TOO_MANY_DATA) {
+                printf("OK\n");
+        } else {
+                printf("FAILED\n");
+        }
+
+        ret = preset_multiple_registers(&mb_param, SLAVE,
+                                        UT_HOLDING_REGISTERS_ADDRESS,
+                                        MAX_REGISTERS + 1,
+                                        tab_rp_registers);
+        printf("* preset_multiple_registers: ");
+        if (ret == TOO_MANY_DATA) {
+                printf("OK\n");
+        } else {
+                printf("FAILED\n");
+        }
+
 close:
         /* Free the memory */
         free(tab_rp_status);