From 3f613c17c1340bf30c5345fff8981e2367c2cb79 Mon Sep 17 00:00:00 2001
From: Chris Jones <christopher.jones@arm.com>
Date: Wed, 31 Mar 2021 09:34:22 +0100
Subject: [PATCH] Improve mbedtls_test_err_add_check documentation

Improve and clarify error messages and comments when checking error codes.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
---
 tests/include/test/helpers.h | 10 ++++++++--
 tests/src/helpers.c          | 28 +++++++++++++++++++++-------
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h
index a26f1eeda9..9bfe08547c 100644
--- a/tests/include/test/helpers.h
+++ b/tests/include/test/helpers.h
@@ -280,9 +280,15 @@ void mbedtls_test_mutex_usage_check( void );
 
 #if defined(MBEDTLS_TEST_HOOKS)
 /**
- * \brief   Check that a pure high-level error code is being combined with a
- *          pure low-level error code as otherwise the resultant error code
+ * \brief   Check that only a pure high-level error code is being combined with
+ *          a pure low-level error code as otherwise the resultant error code
  *          would be corrupted.
+ *
+ * \note    Both high-level and low-level error codes cannot be greater than
+ *          zero however can be zero. If one error code is zero then the
+ *          other error code is returned even if both codes are zero.
+ *
+ * \note    If the check fails, fail the test currently being run.
  */
 void mbedtls_test_err_add_check( int high, int low,
                                  const char *file, int line);
diff --git a/tests/src/helpers.c b/tests/src/helpers.c
index 8319e9004a..8819674093 100644
--- a/tests/src/helpers.c
+++ b/tests/src/helpers.c
@@ -287,27 +287,41 @@ void mbedtls_param_failed( const char *failure_condition,
 void mbedtls_test_err_add_check( int high, int low,
                                  const char *file, int line )
 {
-    if ( high > -0x1000 )
+    /* Error codes are always negative (a value of zero is a success) however
+     * their positive opposites can be easier to understand. The following
+     * examples given in comments have been made positive for ease of
+     * understanding. The structure of an error code is such:
+     *
+     *                                                shhhhhhhllllllll
+     *
+     * s = sign bit.
+     * h = high level error code (includes high and module error codes).
+     * l = low level error code.
+     */
+    if ( high > -0x1000 )               // high < 0001000000000000
     {
         mbedtls_test_fail( "'high' is not a high-level error code",
                             line, file );
     }
-    else if ( high < -0x7F80 )
+    else if ( high < -0x7F80 )          // high > 0111111110000000
     {
-        mbedtls_test_fail( "'high' is greater than 16-bits", line, file );
+        mbedtls_test_fail( "'high' error code is greater than 15 bits",
+                            line, file );
     }
-    else if ( ( high & 0x7F ) != 0 )
+    else if ( ( high & 0x7F ) != 0 )    // high & 0000000011111111
     {
         mbedtls_test_fail( "'high' contains a low-level error code",
                             line, file );
     }
-    else if ( low < -0x007F )
+    else if ( low < -0x007F )           // low >  0000000001111111
     {
-        mbedtls_test_fail( "'low' is greater than 8-bits", line, file );
+        mbedtls_test_fail( "'low' error code is greater than 7 bits",
+                            line, file );
     }
     else if ( low > 0 )
     {
-        mbedtls_test_fail( "'low' is zero or greater", line, file );
+        mbedtls_test_fail( "'low' error code is greater than zero",
+                            line, file );
     }
 }
 #endif /* MBEDTLS_TEST_HOOKS */