Fix off-by-one error in dds_strretcode (#270)

Signed-off-by: Erik Boasson <eb@ilities.com>

src/ddsrt/include/dds/ddsrt/retcode.h

@@ -28,20 +28,20 @@ typedef int32_t dds_return_t;
  * @name DDS_Error_Type
  * @{
  */
-#define DDS_RETCODE_OK                   0 /**< Success */
-#define DDS_RETCODE_ERROR                -1 /**< Non specific error */
-#define DDS_RETCODE_UNSUPPORTED          -2 /**< Feature unsupported */
-#define DDS_RETCODE_BAD_PARAMETER        -3 /**< Bad parameter value */
-#define DDS_RETCODE_PRECONDITION_NOT_MET -4 /**< Precondition for operation not met */
-#define DDS_RETCODE_OUT_OF_RESOURCES     -5 /**< When an operation fails because of a lack of resources */
-#define DDS_RETCODE_NOT_ENABLED          -6 /**< When a configurable feature is not enabled */
-#define DDS_RETCODE_IMMUTABLE_POLICY     -7 /**< When an attempt is made to modify an immutable policy */
-#define DDS_RETCODE_INCONSISTENT_POLICY  -8 /**< When a policy is used with inconsistent values */
-#define DDS_RETCODE_ALREADY_DELETED      -9 /**< When an attempt is made to delete something more than once */
-#define DDS_RETCODE_TIMEOUT              -10 /**< When a timeout has occurred */
-#define DDS_RETCODE_NO_DATA              -11 /**< When expected data is not provided */
-#define DDS_RETCODE_ILLEGAL_OPERATION    -12 /**< When a function is called when it should not be */
-#define DDS_RETCODE_NOT_ALLOWED_BY_SECURITY -13 /**< When credentials are not enough to use the function */
+#define DDS_RETCODE_OK                   (0) /**< Success */
+#define DDS_RETCODE_ERROR                (-1) /**< Non specific error */
+#define DDS_RETCODE_UNSUPPORTED          (-2) /**< Feature unsupported */
+#define DDS_RETCODE_BAD_PARAMETER        (-3) /**< Bad parameter value */
+#define DDS_RETCODE_PRECONDITION_NOT_MET (-4) /**< Precondition for operation not met */
+#define DDS_RETCODE_OUT_OF_RESOURCES     (-5) /**< When an operation fails because of a lack of resources */
+#define DDS_RETCODE_NOT_ENABLED          (-6) /**< When a configurable feature is not enabled */
+#define DDS_RETCODE_IMMUTABLE_POLICY     (-7) /**< When an attempt is made to modify an immutable policy */
+#define DDS_RETCODE_INCONSISTENT_POLICY  (-8) /**< When a policy is used with inconsistent values */
+#define DDS_RETCODE_ALREADY_DELETED      (-9) /**< When an attempt is made to delete something more than once */
+#define DDS_RETCODE_TIMEOUT              (-10) /**< When a timeout has occurred */
+#define DDS_RETCODE_NO_DATA              (-11) /**< When expected data is not provided */
+#define DDS_RETCODE_ILLEGAL_OPERATION    (-12) /**< When a function is called when it should not be */
+#define DDS_RETCODE_NOT_ALLOWED_BY_SECURITY (-13) /**< When credentials are not enough to use the function */
 
 
 /* Extended return codes are not in the DDS specification and are meant

src/ddsrt/src/retcode.c

@@ -30,6 +30,7 @@ static const char *retcodes[] = {
 };
 
 static const char *xretcodes[] = {
+  "Unknown return code",
   "Operation in progress",
   "Try again",
   "Interrupted",
@@ -50,7 +51,11 @@ const char *dds_strretcode (dds_return_t rc)
   /* Retcodes used to be positive, but return values from the API would be a negative
      and so there are/were/may be places outside the core library where dds_strretcode
      is called with a -N for N a API return value, so ... play it safe and use the
-     magnitude */
+     magnitude.  Specially handle INT32_MIN to avoid undefined behaviour on integer
+     overflow. */
+  if (rc == INT32_MIN)
+    return xretcodes[0];
+
   if (rc < 0)
     rc = -rc;
   if (rc >= 0 && rc < nretcodes)
@@ -58,5 +63,5 @@ const char *dds_strretcode (dds_return_t rc)
   else if (rc >= (-DDS_XRETCODE_BASE) && rc < (-DDS_XRETCODE_BASE) + nxretcodes)
     return xretcodes[rc - (-DDS_XRETCODE_BASE)];
   else
-    return "Unknown return code";
+    return xretcodes[0];
 }

src/ddsrt/tests/CMakeLists.txt

@@ -24,6 +24,7 @@ list(APPEND sources
   "string.c"
   "log.c"
   "random.c"
+  "retcode.c"
   "strlcpy.c"
   "socket.c"
   "select.c")

src/ddsrt/tests/retcode.c

@@ -0,0 +1,59 @@
+/*
+ * Copyright(c) 2006 to 2018 ADLINK Technology Limited and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0, or the Eclipse Distribution License
+ * v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
+ */
+#include <string.h>
+
+#include "CUnit/Theory.h"
+#include "dds/ddsrt/retcode.h"
+
+CU_TheoryDataPoints(ddsrt_retcode, unknown) = {
+  CU_DataPoints(dds_return_t,
+                DDS_RETCODE_NOT_ALLOWED_BY_SECURITY-1,
+                -(DDS_RETCODE_NOT_ALLOWED_BY_SECURITY-1),
+                DDS_XRETCODE_BASE,
+                -DDS_XRETCODE_BASE,
+                DDS_RETCODE_NOT_FOUND-1,
+                -(DDS_RETCODE_NOT_FOUND-1),
+                INT32_MAX,
+                -INT32_MAX,
+                INT32_MIN)
+};
+
+CU_Theory((dds_return_t ret), ddsrt_retcode, unknown)
+{
+  CU_ASSERT_STRING_EQUAL(dds_strretcode(ret), "Unknown return code");
+}
+
+CU_TheoryDataPoints(ddsrt_retcode, spotchecks) = {
+  CU_DataPoints(dds_return_t,
+                DDS_RETCODE_OK,
+                -DDS_RETCODE_OK,
+                DDS_RETCODE_NOT_ALLOWED_BY_SECURITY,
+                -DDS_RETCODE_NOT_ALLOWED_BY_SECURITY,
+                DDS_RETCODE_IN_PROGRESS,
+                -DDS_RETCODE_IN_PROGRESS,
+                DDS_RETCODE_NOT_FOUND,
+                -DDS_RETCODE_NOT_FOUND),
+  CU_DataPoints(const char *,
+                "Success",
+                "Success",
+                "Not Allowed By Security",
+                "Not Allowed By Security",
+                "Operation in progress",
+                "Operation in progress",
+                "Not found",
+                "Not found")
+};
+
+CU_Theory((dds_return_t ret, const char *exp), ddsrt_retcode, spotchecks)
+{
+  CU_ASSERT_STRING_EQUAL(dds_strretcode(ret), exp);
+}