From c49b88b066f10203fb94ebd91a498bc259cb8c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Luis=20Lafuente?= Date: Sun, 25 Feb 2024 14:20:57 +0100 Subject: [PATCH] C API: update docs based on PR feedback --- doc/external-api/README.md | 2 +- src/libexpr/c/nix_api_expr.h | 2 +- src/libexpr/c/nix_api_value.h | 5 +++++ src/libstore/c/nix_api_store.cc | 2 +- src/libstore/c/nix_api_store.h | 13 +++++++------ tests/unit/libstore-support/tests/nix_api_store.hh | 2 +- tests/unit/libstore/nix_api_store.cc | 4 ++-- 7 files changed, 18 insertions(+), 12 deletions(-) diff --git a/doc/external-api/README.md b/doc/external-api/README.md index 24118f9f0..1d7344ddd 100644 --- a/doc/external-api/README.md +++ b/doc/external-api/README.md @@ -54,7 +54,7 @@ int main() { nix_gc_decref(NULL, value); nix_state_free(state); - nix_store_unref(store); + nix_store_free(store); return 0; } ``` diff --git a/src/libexpr/c/nix_api_expr.h b/src/libexpr/c/nix_api_expr.h index 7f32140a0..7504b5d7a 100644 --- a/src/libexpr/c/nix_api_expr.h +++ b/src/libexpr/c/nix_api_expr.h @@ -18,7 +18,7 @@ * * nix_gc_decref(NULL, value); * nix_state_free(state); - * nix_store_unref(store); + * nix_store_free(store); * return 0; * } * @endcode diff --git a/src/libexpr/c/nix_api_value.h b/src/libexpr/c/nix_api_value.h index a9a640231..64c0c367a 100644 --- a/src/libexpr/c/nix_api_value.h +++ b/src/libexpr/c/nix_api_value.h @@ -269,6 +269,11 @@ const char * nix_get_attr_name_byidx(nix_c_context * context, const Value * valu /**@}*/ /** @name Initializers + * + * Values are typically "returned" by initializing already allocated memory that serves as the return value. + * For this reason, the construction of values is not tied their allocation. + * Nix is a language with immutable values. Respect this property by only initializing Values once; and only initialize + * Values that are meant to be initialized by you. Failing to adhere to these rules may lead to undefined behavior. */ /**@{*/ /** @brief Set boolean value diff --git a/src/libstore/c/nix_api_store.cc b/src/libstore/c/nix_api_store.cc index 656eb2ae7..d6602471d 100644 --- a/src/libstore/c/nix_api_store.cc +++ b/src/libstore/c/nix_api_store.cc @@ -50,7 +50,7 @@ Store * nix_store_open(nix_c_context * context, const char * uri, const char *** NIXC_CATCH_ERRS_NULL } -void nix_store_unref(Store * store) +void nix_store_free(Store * store) { delete store; } diff --git a/src/libstore/c/nix_api_store.h b/src/libstore/c/nix_api_store.h index 9c5e524e5..e6d88026b 100644 --- a/src/libstore/c/nix_api_store.h +++ b/src/libstore/c/nix_api_store.h @@ -48,23 +48,24 @@ nix_err nix_init_plugins(nix_c_context * context); /** * @brief Open a nix store + * Store instances may share state and resources behind the scenes. * @param[out] context Optional, stores error information * @param[in] uri URI of the nix store, copied * @param[in] params optional, array of key-value pairs, {{"endpoint", * "https://s3.local"}} - * @return ref-counted Store pointer, NULL in case of errors - * @see nix_store_unref + * @return a Store pointer, NULL in case of errors + * @see nix_store_free */ Store * nix_store_open(nix_c_context *, const char * uri, const char *** params); /** - * @brief Unref a nix store + * @brief Deallocate a nix store and free any resources if not also held by other Store instances. * * Does not fail. - * It'll be closed and deallocated when all references are gone. - * @param[in] builder the store to unref + * + * @param[in] store the store to free */ -void nix_store_unref(Store * store); +void nix_store_free(Store * store); /** * @brief get the URI of a nix store diff --git a/tests/unit/libstore-support/tests/nix_api_store.hh b/tests/unit/libstore-support/tests/nix_api_store.hh index e762a3ca8..34d467d49 100644 --- a/tests/unit/libstore-support/tests/nix_api_store.hh +++ b/tests/unit/libstore-support/tests/nix_api_store.hh @@ -27,7 +27,7 @@ public: }; ~nix_api_store_test() override { - nix_store_unref(store); + nix_store_free(store); for (auto & path : fs::recursive_directory_iterator(nixStoreDir)) { fs::permissions(path, fs::perms::owner_all); diff --git a/tests/unit/libstore/nix_api_store.cc b/tests/unit/libstore/nix_api_store.cc index e093e9d15..54daf927a 100644 --- a/tests/unit/libstore/nix_api_store.cc +++ b/tests/unit/libstore/nix_api_store.cc @@ -69,7 +69,7 @@ TEST_F(nix_api_util_context, nix_store_open_dummy) nix_store_get_version(ctx, store, value, 256); ASSERT_STREQ("", value); - nix_store_unref(store); + nix_store_free(store); } TEST_F(nix_api_util_context, nix_store_open_invalid) @@ -78,7 +78,7 @@ TEST_F(nix_api_util_context, nix_store_open_invalid) Store * store = nix_store_open(ctx, "invalid://", nullptr); ASSERT_EQ(NIX_ERR_NIX_ERROR, ctx->last_err_code); ASSERT_EQ(nullptr, store); - nix_store_unref(store); + nix_store_free(store); } TEST_F(nix_api_store_test, nix_store_is_valid_path_not_in_store)