From 2dc7598779178188fddf87e6da940636c9d67b2e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 14 Jun 2024 16:30:34 +0200 Subject: [PATCH 1/2] C API: Add nix_clear_err --- src/libutil-c/nix_api_util.cc | 6 ++++++ src/libutil-c/nix_api_util.h | 29 ++++++++++++++++++++++++++--- tests/unit/libutil/nix_api_util.cc | 3 +++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/libutil-c/nix_api_util.cc b/src/libutil-c/nix_api_util.cc index 0a9b49345..c976b1815 100644 --- a/src/libutil-c/nix_api_util.cc +++ b/src/libutil-c/nix_api_util.cc @@ -57,6 +57,12 @@ nix_err nix_set_err_msg(nix_c_context * context, nix_err err, const char * msg) return err; } +void nix_clear_err(nix_c_context * context) +{ + if (context) + context->last_err_code = NIX_OK; +} + const char * nix_version_get() { return PACKAGE_VERSION; diff --git a/src/libutil-c/nix_api_util.h b/src/libutil-c/nix_api_util.h index e0ca04e69..9a9f1d98d 100644 --- a/src/libutil-c/nix_api_util.h +++ b/src/libutil-c/nix_api_util.h @@ -221,7 +221,9 @@ const char * nix_version_get(); * @param[out] n optional: a pointer to an unsigned int that is set to the * length of the error. * @return nullptr if no error message was ever set, - * a borrowed pointer to the error message otherwise. + * a borrowed pointer to the error message otherwise, which is valid + * until the next call to a Nix function, or until the context is + * destroyed. */ const char * nix_err_msg(nix_c_context * context, const nix_c_context * ctx, unsigned int * n); @@ -282,13 +284,34 @@ nix_err nix_err_code(const nix_c_context * read_context); * * All other use is internal to the API. * - * @param context context to write the error message to, or NULL + * @param context context to write the error message to, required unless C++ exceptions are supported * @param err The error code to set and return - * @param msg The error message to set. + * @param msg The error message to set. This string is copied. * @returns the error code set */ nix_err nix_set_err_msg(nix_c_context * context, nix_err err, const char * msg); +/** + * @brief Clear the error message from a nix context. + * + * This is performed implicitly by all functions that accept a context, so + * this won't be necessary in most cases. + * However, if you want to clear the error message without calling another + * function, you can use this. + * + * Example use case: a higher order function that helps with error handling, + * to make it more robust in the following scenario: + * + * 1. A previous call failed, and the error was caught and handled. + * 2. The context is reused with our error handling helper function. + * 3. The callback passed to the helper function doesn't actually make a call to + * a Nix function. + * 4. The handled error is raised again, from an unrelated call. + * + * This failure can be avoided by clearing the error message after handling it. + */ +void nix_clear_err(nix_c_context * context); + /** * @} */ diff --git a/tests/unit/libutil/nix_api_util.cc b/tests/unit/libutil/nix_api_util.cc index d2999f55b..052d0c533 100644 --- a/tests/unit/libutil/nix_api_util.cc +++ b/tests/unit/libutil/nix_api_util.cc @@ -31,6 +31,9 @@ TEST_F(nix_api_util_context, nix_context_error) } ASSERT_EQ(ctx->last_err_code, NIX_ERR_UNKNOWN); ASSERT_EQ(*ctx->last_err, err_msg_ref); + + nix_clear_err(ctx); + ASSERT_EQ(ctx->last_err_code, NIX_OK); } TEST_F(nix_api_util_context, nix_set_err_msg) From 61381c9964c69afd720264272a0f42d298d6616c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 14 Jun 2024 16:36:23 +0200 Subject: [PATCH 2/2] C API: Make nix_err_msg treat NIX_OK as having no message The documentation "solved" this by specifying a precondition, but let's just make it more robust, and not leak irrelevant messages that might linger. We don't clear the message when clearing the status, in order to keep clearing fast; see last_err field doc. --- src/libutil-c/nix_api_util.cc | 2 +- src/libutil-c/nix_api_util_internal.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libutil-c/nix_api_util.cc b/src/libutil-c/nix_api_util.cc index c976b1815..cf468c8cd 100644 --- a/src/libutil-c/nix_api_util.cc +++ b/src/libutil-c/nix_api_util.cc @@ -112,7 +112,7 @@ const char * nix_err_msg(nix_c_context * context, const nix_c_context * read_con { if (context) context->last_err_code = NIX_OK; - if (read_context->last_err) { + if (read_context->last_err && read_context->last_err_code != NIX_OK) { if (n) *n = read_context->last_err->size(); return read_context->last_err->c_str(); diff --git a/src/libutil-c/nix_api_util_internal.h b/src/libutil-c/nix_api_util_internal.h index aa829feaf..7fa4252ac 100644 --- a/src/libutil-c/nix_api_util_internal.h +++ b/src/libutil-c/nix_api_util_internal.h @@ -10,6 +10,7 @@ struct nix_c_context { nix_err last_err_code = NIX_OK; + /** The last error message. Always check last_err_code. This may not have been cleared, so that clearing is fast. */ std::optional last_err = {}; std::optional info = {}; std::string name = "";