modbus_strerror_r() introduced to avoid MT-Unsafe functions (#468)

configure.ac

@@ -104,7 +104,10 @@ AC_CHECK_DECLS([__CYGWIN__])
 AC_SEARCH_LIBS(accept, network socket)
 
 # Checks for library functions.
-AC_CHECK_FUNCS([accept4 getaddrinfo gettimeofday inet_pton inet_ntop select socket strerror strlcpy])
+AC_CHECK_FUNCS([accept4 getaddrinfo gettimeofday inet_pton inet_ntop select socket strerror_r strlcpy])
+
+# Check which flavour of strerror_r is available on the target
+AC_FUNC_STRERROR_R
 
 # Required for MinGW with GCC v4.8.1 on Win7
 AC_DEFINE(WINVER, 0x0501, _)

docs/index.md

@@ -245,9 +245,11 @@ shall return either a NULL value (if returning a pointer) or a negative value
 (if returning an integer), and the actual error code shall be stored in the
 `errno` variable.
 
-The *modbus_strerror()* function is provided to translate libmodbus-specific
-error codes into error message strings; for details refer to
-[modbus_strerror](modbus_strerror.md).
+The *modbus_strerror_r()* function is provided to translate libmodbus-specific
+error codes into error message strings; for details refer to:
+
+- [modbus_strerror_r](modbus_strerror_r.md)
+- [modbus_strerror](modbus_strerror.md) **deprecated**
 
 ## Miscellaneous
 

docs/modbus_close.md

@@ -26,7 +26,9 @@ modbus_t *ctx;
 
 ctx = modbus_new_tcp("127.0.0.1", 502);
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno), buf, sizeof(buf));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_connect.md

@@ -29,7 +29,9 @@ modbus_t *ctx;
 
 ctx = modbus_new_tcp("127.0.0.1", 502);
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno), buf, sizeof(buf));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_mapping_new.md

@@ -45,9 +45,10 @@ mb_mapping = modbus_mapping_new(
     INPUT_REGISTERS_ADDRESS + INPUT_REGISTERS_NB
 );
 if (mb_mapping == NULL) {
+    char buf[128];
     fprintf(
         stderr, "Failed to allocate the mapping: %s\n",
-        modbus_strerror(errno)
+        modbus_strerror_r(errno, buf, sizeof(buf))
     );
     modbus_free(ctx);
     return -1;

docs/modbus_mapping_new_start_address.md

@@ -70,9 +70,10 @@ mb_mapping = modbus_mapping_new_start_address(
     INPUT_REGISTERS_ADDRESS, INPUT_REGISTERS_NB
 );
 if (mb_mapping == NULL) {
+    char buf[128];
     fprintf(
         stderr, "Failed to allocate the mapping: %s\n",
-        modbus_strerror(errno)
+        modbus_strerror_r(errno, buf, sizeof(buf))
     );
     modbus_free(ctx);
     return -1;

docs/modbus_new_rtu.md

@@ -86,7 +86,9 @@ if (ctx == NULL) {
 }
 
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_new_tcp.md

@@ -48,7 +48,9 @@ if (ctx == NULL) {
 }
 
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_new_tcp_pi.md

@@ -49,7 +49,9 @@ if (ctx == NULL) {
 }
 
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_read_registers.md

@@ -40,14 +40,17 @@ int i;
 
 ctx = modbus_new_tcp("127.0.0.1", 1502);
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }
 
 rc = modbus_read_registers(ctx, 0, 10, tab_reg);
 if (rc == -1) {
-    fprintf(stderr, "%s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "%s\n", modbus_strerror_r(errno, buf, sizeof(buf));
     return -1;
 }
 

docs/modbus_rtu_set_rts.md

@@ -49,14 +49,17 @@ modbus_rtu_set_serial_mode(ctx, MODBUS_RTU_RS485);
 modbus_rtu_set_rts(ctx, MODBUS_RTU_RTS_UP);
 
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }
 
 rc = modbus_read_registers(ctx, 0, 7, tab_reg);
 if (rc == -1) {
-    fprintf(stderr, "%s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "%s\n", modbus_strerror_r(errno, buf, sizeof(buf)));
     return -1;
 }
 

docs/modbus_send_raw_request.md

@@ -40,7 +40,9 @@ uint8_t rsp[MODBUS_TCP_MAX_ADU_LENGTH];
 
 ctx = modbus_new_tcp("127.0.0.1", 1502);
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_set_slave.md

@@ -66,7 +66,9 @@ if (rc == -1) {
 }
 
 if (modbus_connect(ctx) == -1) {
-    fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+    char buf[128];
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
     modbus_free(ctx);
     return -1;
 }

docs/modbus_strerror.md

@@ -10,6 +10,9 @@ modbus_strerror - return the error message
 const char *modbus_strerror(int errnum);
 ```
 
+Warning, this function is *deprecated* since libmodbus **tbd** and has been
+replaced by *modbus_strerror_r()*.
+
 ## Description
 
 The *modbus_strerror()* function shall return a pointer to an error message

docs/modbus_strerror_r.md

@@ -0,0 +1,50 @@
+# modbus_strerror_r
+
+## Name
+
+modbus_strerror_r - return the error message
+
+## Synopsis
+
+```c
+const char *modbus_strerro_r(int errnum, char buf[.buflen], size_t buflen);
+```
+
+## Description
+
+The *modbus_strerror_r()* function is a reentrant replacement for
+*modbus_strerror()*.
+
+The *modbus_strerror_r()* function returns a pointer to a string containing
+the error message. This may either be a pointer to a string that the
+function stores in *buf*, or a pointer to some (immutable) static string
+(in which case *buf* is unused). If the function stores a string in *buf*,
+then at most *buflen* bytes are stored. The string may be truncated if
+*buflen* is tool small and errnum is unknown. The string always includes a
+terminating null byte ('\0').
+
+As libmodbus defines additional error numbers over and above those defined
+by the operating system, applications should use *modbus_strerror_r()* in
+preference to the standard *strerror_r()* function.
+
+## Return value
+
+The *modbus_strerror()* function shall return a pointer to an error message
+string.
+
+## Errors
+
+No errors are defined.
+
+## Example
+
+Display an error message when a Modbus connection cannot be established
+
+```c
+if (modbus_connect(ctx) == -1) {
+    char buf[128]
+    fprintf(stderr, "Connection failed: %s\n",
+        modbus_strerror_r(errno, buf, sizeof(buf)));
+    abort();
+}
+```

src/modbus-rtu.c

@@ -12,6 +12,7 @@
 #ifndef _MSC_VER
 #include <unistd.h>
 #endif
+#include "modbus.h"
 #include "modbus-private.h"
 #include <assert.h>
 
@@ -649,10 +650,11 @@ static int _modbus_rtu_connect(modbus_t *ctx)
     ctx->s = open(ctx_rtu->device, flags);
     if (ctx->s < 0) {
         if (ctx->debug) {
+            char buf[128];
             fprintf(stderr,
                     "ERROR Can't open the device %s (%s)\n",
                     ctx_rtu->device,
-                    strerror(errno));
+                    modbus_strerror_r(errno, buf, sizeof(buf)));
         }
         return -1;
     }

src/modbus.c

@@ -41,7 +41,9 @@ typedef enum {
     _STEP_DATA
 } _step_t;
 
-const char *modbus_strerror(int errnum)
+// returns NULL if the error belongs to the operating system, otherwise a
+// pointer to the error message.
+static const char *libmodbus_specific_error_text(int errnum)
 {
     switch (errnum) {
     case EMBXILFUN:
@@ -75,14 +77,82 @@ const char *modbus_strerror(int errnum)
     case EMBBADSLAVE:
         return "Response not from requested slave";
     default:
-        return strerror(errnum);
+        return NULL;
     }
 }
 
+const char *modbus_strerror(int errnum)
+{
+    const char *s = libmodbus_specific_error_text(errnum);
+
+    if (s) {
+        return s;
+    }
+    return strerror(errnum);
+}
+
+const char *modbus_strerror_r(int errnum, char *buf, size_t buflen)
+{
+    const char *s = libmodbus_specific_error_text(errnum);
+
+    if (s) {
+        return s;
+    }
+
+#if HAVE_STRERROR_R
+ /*
+  * The glibc provides 2 different versions of strerror_r.
+  *
+  * XSI-compliant version:
+  *     int strerror_r(int errnum, char *buf, size_t buflen);
+  *
+  * GNU-specific version:
+  *     char *strerror_r(int errnum, char *buf, size_t buflen);
+  *
+  * See "man 3 strerror_r" for details.
+  */
+#if STRERROR_R_CHAR_P
+    /* GNU-specific strerror_r() */
+    /*
+     * The statement below will hopefully result in a compiler warning if
+     * the XSI-compliant implementation is provided instead of the GNU-specific
+     * version (although detected by configure).
+     */
+    return strerror_r(errnum, buf, buflen);
+#else
+    /* XSI-complaint strerror_r() */
+    /*
+     * Neither POSIX nor the Linux man page specify the buffer content when
+     * strerror_r() completes unsuccessfully. Therefore, we initialize the
+     * buffer with '\0' before we call strerror_r(), and manually terminate
+     * the string in case strerror_r() signals an error.
+     */
+    memset(buf, 0, buflen);
+
+    /*
+     * The statement below will hopefully result in a compiler warning if
+     * the GNU-specific version is provided instead of the XSI-compliant
+     * implementation.
+     */
+    int res = strerror_r(errnum, buf, buflen);
+    if (res != 0)
+        buf[buflen - 1] = 0;
+
+    return buf;
+
+#endif /* STRERROR_R_CHAR_P */
+#else
+    /* fallback in case strerror_r() is not available */
+    snprintf(buf, buflen, "error %d", errnum);
+    return buf;
+#endif /* HAVE_STRERROR_R */
+}
+
 void _error_print(modbus_t *ctx, const char *context)
 {
     if (ctx->debug) {
-        fprintf(stderr, "ERROR %s", modbus_strerror(errno));
+        char buf[128];
+        fprintf(stderr, "ERROR %s", modbus_strerror_r(errno, buf, sizeof(buf)));
         if (context != NULL) {
             fprintf(stderr, ": %s\n", context);
         } else {

src/modbus.h

@@ -216,6 +216,7 @@ MODBUS_API int modbus_flush(modbus_t *ctx);
 MODBUS_API int modbus_set_debug(modbus_t *ctx, int flag);
 
 MODBUS_API const char *modbus_strerror(int errnum);
+MODBUS_API const char *modbus_strerror_r(int errnum, char *buf, size_t buflen);
 
 MODBUS_API int modbus_read_bits(modbus_t *ctx, int addr, int nb, uint8_t *dest);
 MODBUS_API int modbus_read_input_bits(modbus_t *ctx, int addr, int nb, uint8_t *dest);

tests/bandwidth-client.c

@@ -76,7 +76,9 @@ int main(int argc, char *argv[])
         modbus_set_slave(ctx, 1);
     }
     if (modbus_connect(ctx) == -1) {
-        fprintf(stderr, "Connection failed: %s\n", modbus_strerror(errno));
+        char buf[128];
+        fprintf(stderr, "Connection failed: %s\n",
+            modbus_strerror_r(errno, buf, sizeof(buf)));
         modbus_free(ctx);
         return -1;
     }
@@ -96,7 +98,9 @@ int main(int argc, char *argv[])
     for (i = 0; i < n_loop; i++) {
         rc = modbus_read_bits(ctx, 0, nb_points, tab_bit);
         if (rc == -1) {
-            fprintf(stderr, "%s\n", modbus_strerror(errno));
+            char buf[128];
+            fprintf(stderr, "%s\n",
+                modbus_strerror_r(errno, buf, sizeof(buf)));
             return -1;
         }
     }
@@ -133,7 +137,8 @@ int main(int argc, char *argv[])
     for (i = 0; i < n_loop; i++) {
         rc = modbus_read_registers(ctx, 0, nb_points, tab_reg);
         if (rc == -1) {
-            fprintf(stderr, "%s\n", modbus_strerror(errno));
+            char buf[128];
+            fprintf(stderr, "%s\n", modbus_strerror_r(errno, buf, sizeof(buf)));
             return -1;
         }
     }
@@ -171,7 +176,8 @@ int main(int argc, char *argv[])
         rc = modbus_write_and_read_registers(
             ctx, 0, nb_points, tab_reg, 0, nb_points, tab_reg);
         if (rc == -1) {
-            fprintf(stderr, "%s\n", modbus_strerror(errno));
+            char buf[128];
+            fprintf(stderr, "%s\n", modbus_strerror_r(errno, buf, sizeof(buf)));
             return -1;
         }
     }