Start error handling cleanup and rewrite.

Error handling and hte underlying functions and macros that support it
will probably change a bit more as it goes along.  Trying to strike
the right balance between having the main code be readable and having
the underlying support code be at least comprehensible and
straightforward to review.

Also need to address current over-use of CKR_FUNCTION_FAILED.

1 files changed, 94 insertions, 57 deletions

diff --git a/pkcs11.c b/pkcs11.c
index e08b9ca..3e47c67 100644
--- a/pkcs11.c
+++ b/pkcs11.c
@@ -302,23 +302,81 @@ static pid_t initialized_pid;
  * Error checking for libhal calls.
  */
 
+#define hal_whine(_expr_)            (_hal_whine((_expr_), #_expr_, __FILE__, __LINE__, HAL_OK))
+#define hal_whine_allow(_expr_, ...) (_hal_whine((_expr_), #_expr_, __FILE__, __LINE__, __VA_ARGS__, HAL_OK))
+#define hal_check(_expr_)            (hal_whine(_expr_) == HAL_OK)
+
 #if DEBUG_HAL
 
-static int _hal_check(const hal_error_t err, const char * const expr, const char * const file, const unsigned line)
+static inline hal_error_t _hal_whine(const hal_error_t err,
+                                     const char * const expr,
+                                     const char * const file,
+                                     const unsigned line, ...)
 {
-  if (err == HAL_OK)
-    return 1;
-  fprintf(stderr, "\n%s:%u: %s returned %s\n", file, line, expr, hal_error_string(err));
-  return 0;
+  va_list ap;
+  int ok = 0;
+  hal_error_t code;
+
+  va_start(ap, line)
+    do {
+      code = va_arg(ap, hal_error_t);
+      ok |= (err == code);
+    } while (code != HAL_OK);
+  va_end(ap);
+
+  if (!ok)
+    fprintf(stderr, "\n%s:%u: %s returned %s\n", file, line, expr, hal_error_string(err));
+
+  return err;
 }
 
-#define hal_check(_expr_) (_hal_check((_expr_), #_expr_, __FILE__, __LINE__))
+#else /* DEBUG_HAL */
+
+#define _hal_whine(_expr_, ...) (_expr_)
+
+#endif /* DEBUG_HAL */
+
+/*
+ * Error translation fun for the entire family!
+ */
 
-#else  /* DEBUG_HAL */
+#if DEBUG_PKCS11 || DEBUG_HAL
 
-#define hal_check(_expr_) ((_expr_) == HAL_OK)
+#define hal_p11_error_case(_hal_err_, _p11_err_) \
+  case _hal_err_: fprintf(stderr, "\n%s:%u: Mapping %s to %s\n", file, line, #_hal_err_, #_p11_err_); return _p11_err_;
+
+#else
+
+#define hal_p11_error_case(_hal_err_, _p11_err_) \
+  case _hal_err_: return _p11_err_;
+
+#endif
+
+#define p11_error_from_hal(_hal_err_) \
+  (_p11_error_from_hal((_hal_err_), __FILE__, __LINE__))
+
+#define p11_whine_from_hal(_expr_) \
+  (_p11_error_from_hal(_hal_whine((_expr_), #_expr_, __FILE__, __LINE__, HAL_OK), __FILE__, __LINE__))
+
+static CK_RV _p11_error_from_hal(const hal_error_t err, const char * const file, const unsigned line)
+{
+  switch (err) {
+    hal_p11_error_case(HAL_ERROR_PIN_INCORRECT,         CKR_PIN_INCORRECT);
+    hal_p11_error_case(HAL_ERROR_INVALID_SIGNATURE,     CKR_SIGNATURE_INVALID);
+
+    /*
+     * More here later, first see if this compiles.
+     */
+
+  default:
+#if DEBUG_PKCS11 || DEBUG_HAL
+    fprintf(stderr, "\n%s:%u: Mapping unhandled HAL error to CKR_FUNCTION_FAILED\n", file, line);
+#endif
+    return CKR_FUNCTION_FAILED;
+  }
+}
 
-#endif  /* DEBUG_HAL */
+#undef hal_p11_error_case
 
 
 
@@ -1590,8 +1648,12 @@ static int p11_object_get_pkey_handle(const p11_session_t * const session,
   switch (sqlite3_column_type(q, 1)) {
 
   case SQLITE_BLOB:
-    err = hal_rpc_pkey_find(p11_session_hal_client(session), p11_session_hal_session(session), pkey_handle,
-                            pkey_type, sqlite3_column_blob(q, 1), sqlite3_column_bytes(q, 1), flags);
+    err = hal_whine_allow(hal_rpc_pkey_find(p11_session_hal_client(session),
+                                            p11_session_hal_session(session), pkey_handle,
+                                            pkey_type, sqlite3_column_blob(q, 1),
+                                            sqlite3_column_bytes(q, 1),
+                                            flags),
+                          HAL_ERROR_KEY_NOT_FOUND);
     break;
 
   case SQLITE_NULL:
@@ -1602,29 +1664,17 @@ static int p11_object_get_pkey_handle(const p11_session_t * const session,
     goto fail;
   }
 
-  if (err == HAL_ERROR_KEY_NOT_FOUND) {
-    switch (pkey_type) {
-
-    case HAL_KEY_TYPE_RSA_PUBLIC:
-      if (!p11_object_get_rsa_public_key(session, object_handle, pkey_handle, flags))
-        goto fail;
-      break;
-
-    case HAL_KEY_TYPE_EC_PUBLIC:
-      if (!p11_object_get_ec_public_key(session, object_handle, pkey_handle, flags))
-        goto fail;
-      break;
+  if (err == HAL_OK)
+    ok = 1;
 
-    default:
-      (void) hal_check(err);
-      goto fail;
-    }
-  }
+  else if (err == HAL_ERROR_KEY_NOT_FOUND && pkey_type == HAL_KEY_TYPE_RSA_PUBLIC)
+    ok = p11_object_get_rsa_public_key(session, object_handle, pkey_handle, flags);
 
-  else if (!hal_check(err))
-    goto fail;
+  else if (err == HAL_ERROR_KEY_NOT_FOUND && pkey_type == HAL_KEY_TYPE_EC_PUBLIC)
+    ok = p11_object_get_ec_public_key(session, object_handle, pkey_handle, flags);
 
-  ok = 1;
+  else
+    ok = 0;
 
  fail:
   sqlite3_finalize(q);
@@ -2498,13 +2548,12 @@ static CK_RV sign_hal_rpc(p11_session_t *session,
   if (pSignature != NULL && rv == CKR_BUFFER_TOO_SMALL)
     lose(CKR_BUFFER_TOO_SMALL);
 
-#warning Should pay more attention to translating error codes here
-  if (pSignature != NULL &&
-      !hal_check(hal_rpc_pkey_sign(p11_session_hal_session(session), pkey, session->sign_digest_handle,
-                                   pData, ulDataLen, pSignature, &signature_len, signature_len)))
-    lose(CKR_FUNCTION_FAILED);
-
-  rv = CKR_OK;                  /* Fall through */
+  if (pSignature == NULL)
+    rv = CKR_OK;
+  else
+    rv = p11_whine_from_hal(hal_rpc_pkey_sign(p11_session_hal_session(session), pkey, session->sign_digest_handle,
+                                              pData, ulDataLen, pSignature, &signature_len, signature_len));
+  /* Fall through */
 
  fail:
   hal_rpc_pkey_close(pkey);
@@ -2529,14 +2578,9 @@ static CK_RV verify_hal_rpc(p11_session_t *session,
   if (!p11_object_get_pkey_handle(session, session->verify_key_handle, &pkey))
     lose(CKR_FUNCTION_FAILED);
 
-#warning Should pay more attention to translating error codes here
-
-  if (!hal_check(hal_rpc_pkey_verify(p11_session_hal_session(session), pkey, session->verify_digest_handle,
-                                     pData, ulDataLen, pSignature, ulSignatureLen)))
-    lose(CKR_FUNCTION_FAILED);
- /* lose(CKR_SIGNATURE_INVALID); */
-
-  rv = CKR_OK;                  /* Fall through */
+  rv = p11_whine_from_hal(hal_rpc_pkey_verify(p11_session_hal_session(session), pkey, session->verify_digest_handle,
+                                              pData, ulDataLen, pSignature, ulSignatureLen));
+  /* Fall through */
 
  fail:
   hal_rpc_pkey_close(pkey);
@@ -2978,7 +3022,6 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
   p11_session_t *session;
   hal_user_t user = HAL_USER_NONE;
   CK_RV rv = CKR_OK;
-  hal_error_t err;
 
   mutex_lock_or_return_failure(p11_global_mutex);
 
@@ -3030,13 +3073,8 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
    * Try to log in the HSM.
    */
 
-#warning Might need better error code translation here
-  if (!hal_check((err = hal_rpc_login(client, user, (char *) pPin, ulPinLen)))) {
-    if (err == HAL_ERROR_PIN_INCORRECT)
-      lose(CKR_PIN_INCORRECT);
-    else
-      lose(CKR_FUNCTION_FAILED);
-  }
+  if ((rv = p11_whine_from_hal(hal_rpc_login(client, user, (char *) pPin, ulPinLen))) != CKR_OK)
+    goto fail;
 
   /*
    * If we get here, the PIN was OK.  Update global login state, then
@@ -3097,9 +3135,8 @@ CK_RV C_Logout(CK_SESSION_HANDLE hSession)
 
   assert(p11_session_consistent_login());
 
-#warning Might want better error translation here
-  if (!hal_check(hal_rpc_logout(client)))
-    lose(CKR_FUNCTION_FAILED);
+  if ((rv = p11_whine_from_hal(hal_rpc_logout(client))) != CKR_OK)
+    goto fail;
 
   logged_in_as = not_logged_in;