diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 2550e975a..0366e5020 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -20,7 +20,7 @@ # include "gc_cpp.h" #endif -// Helper function to throw an exception if value is null +// Internal helper functions to check [in] and [out] `Value *` parameters static const nix::Value & check_value_not_null(const Value * value) { if (!value) { @@ -37,6 +37,33 @@ static nix::Value & check_value_not_null(Value * value) return *((nix::Value *) value); } +static const nix::Value & check_value_in(const Value * value) +{ + auto & v = check_value_not_null(value); + if (!v.isValid()) { + throw std::runtime_error("Uninitialized Value"); + } + return v; +} + +static nix::Value & check_value_in(Value * value) +{ + auto & v = check_value_not_null(value); + if (!v.isValid()) { + throw std::runtime_error("Uninitialized Value"); + } + return v; +} + +static nix::Value & check_value_out(Value * value) +{ + auto & v = check_value_not_null(value); + if (v.isValid()) { + throw std::runtime_error("Value already initialized. Variables are immutable"); + } + return v; +} + /** * Helper function to convert calls from nix into C API. * @@ -111,7 +138,7 @@ ValueType nix_get_type(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); using namespace nix; switch (v.type()) { case nThunk: @@ -147,7 +174,7 @@ const char * nix_get_typename(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); auto s = nix::showType(v); return strdup(s.c_str()); } @@ -159,7 +186,7 @@ bool nix_get_bool(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nBool); return v.boolean(); } @@ -171,7 +198,7 @@ nix_err nix_get_string(nix_c_context * context, const Value * value, nix_get_str if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nString); call_nix_get_string_callback(v.c_str(), callback, user_data); } @@ -183,7 +210,7 @@ const char * nix_get_path_string(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nPath); // NOTE (from @yorickvP) // v._path.path should work but may not be how Eelco intended it. @@ -202,7 +229,7 @@ unsigned int nix_get_list_size(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nList); return v.listSize(); } @@ -214,7 +241,7 @@ unsigned int nix_get_attrs_size(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nAttrs); return v.attrs()->size(); } @@ -226,7 +253,7 @@ double nix_get_float(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nFloat); return v.fpoint(); } @@ -238,7 +265,7 @@ int64_t nix_get_int(nix_c_context * context, const Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nInt); return v.integer(); } @@ -250,7 +277,7 @@ ExternalValue * nix_get_external(nix_c_context * context, Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); assert(v.type() == nix::nExternal); return (ExternalValue *) v.external(); } @@ -262,7 +289,7 @@ Value * nix_get_list_byidx(nix_c_context * context, const Value * value, EvalSta if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nList); auto * p = v.listElems()[ix]; nix_gc_incref(nullptr, p); @@ -278,7 +305,7 @@ Value * nix_get_attr_byname(nix_c_context * context, const Value * value, EvalSt if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nAttrs); nix::Symbol s = state->state.symbols.create(name); auto attr = v.attrs()->get(s); @@ -298,7 +325,7 @@ bool nix_has_attr_byname(nix_c_context * context, const Value * value, EvalState if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); assert(v.type() == nix::nAttrs); nix::Symbol s = state->state.symbols.create(name); auto attr = v.attrs()->get(s); @@ -315,7 +342,7 @@ nix_get_attr_byidx(nix_c_context * context, const Value * value, EvalState * sta if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); const nix::Attr & a = (*v.attrs())[i]; *name = ((const std::string &) (state->state.symbols[a.name])).c_str(); nix_gc_incref(nullptr, a.value); @@ -330,7 +357,7 @@ const char * nix_get_attr_name_byidx(nix_c_context * context, const Value * valu if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); const nix::Attr & a = (*v.attrs())[i]; return ((const std::string &) (state->state.symbols[a.name])).c_str(); } @@ -342,7 +369,7 @@ nix_err nix_init_bool(nix_c_context * context, Value * value, bool b) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkBool(b); } NIXC_CATCH_ERRS @@ -354,7 +381,7 @@ nix_err nix_init_string(nix_c_context * context, Value * value, const char * str if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkString(std::string_view(str)); } NIXC_CATCH_ERRS @@ -365,7 +392,7 @@ nix_err nix_init_path_string(nix_c_context * context, EvalState * s, Value * val if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkPath(s->state.rootPath(nix::CanonPath(str))); } NIXC_CATCH_ERRS @@ -376,7 +403,7 @@ nix_err nix_init_float(nix_c_context * context, Value * value, double d) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkFloat(d); } NIXC_CATCH_ERRS @@ -387,7 +414,7 @@ nix_err nix_init_int(nix_c_context * context, Value * value, int64_t i) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkInt(i); } NIXC_CATCH_ERRS @@ -398,7 +425,7 @@ nix_err nix_init_null(nix_c_context * context, Value * value) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkNull(); } NIXC_CATCH_ERRS @@ -422,7 +449,7 @@ nix_err nix_init_external(nix_c_context * context, Value * value, ExternalValue if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); auto r = (nix::ExternalValueBase *) val; v.mkExternal(r); } @@ -469,7 +496,7 @@ nix_err nix_make_list(nix_c_context * context, ListBuilder * list_builder, Value if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkList(list_builder->builder); } NIXC_CATCH_ERRS @@ -480,19 +507,19 @@ nix_err nix_init_primop(nix_c_context * context, Value * value, PrimOp * p) if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkPrimOp((nix::PrimOp *) p); } NIXC_CATCH_ERRS } -nix_err nix_copy_value(nix_c_context * context, Value * value, Value * source) +nix_err nix_copy_value(nix_c_context * context, Value * value, const Value * source) { if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); - auto & s = check_value_not_null(source); + auto & v = check_value_out(value); + auto & s = check_value_in(source); v = s; } NIXC_CATCH_ERRS @@ -503,7 +530,7 @@ nix_err nix_make_attrs(nix_c_context * context, Value * value, BindingsBuilder * if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_out(value); v.mkAttrs(b->builder); } NIXC_CATCH_ERRS @@ -550,7 +577,7 @@ nix_realised_string * nix_string_realise(nix_c_context * context, EvalState * st if (context) context->last_err_code = NIX_OK; try { - auto & v = check_value_not_null(value); + auto & v = check_value_in(value); nix::NixStringContext stringContext; auto rawStr = state->state.coerceToString(nix::noPos, v, stringContext, "while realising a string").toOwned(); nix::StorePathSet storePaths; diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index d8bd77c33..b2b3439ef 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -422,7 +422,7 @@ nix_err nix_init_primop(nix_c_context * context, Value * value, PrimOp * op); * @param[in] source value to copy from * @return error code, NIX_OK on success. */ -nix_err nix_copy_value(nix_c_context * context, Value * value, Value * source); +nix_err nix_copy_value(nix_c_context * context, Value * value, const Value * source); /**@}*/ /** @brief Create a bindings builder diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 7ed3fa5a9..5795f04cf 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -23,6 +23,7 @@ class BindingsBuilder; typedef enum { + tUninitialized = 0, tInt = 1, tBool, tString, @@ -166,7 +167,7 @@ public: struct Value { private: - InternalType internalType; + InternalType internalType = tUninitialized; friend std::string showType(const Value & v); @@ -270,6 +271,7 @@ public: inline ValueType type(bool invalidIsThunk = false) const { switch (internalType) { + case tUninitialized: break; case tInt: return nInt; case tBool: return nBool; case tString: return nString; @@ -294,6 +296,16 @@ public: internalType = newType; } + /** + * A value becomes valid when it is initialized. We don't use this + * in the evaluator; only in the bindings, where the slight extra + * cost is warranted because of inexperienced callers. + */ + inline bool isValid() const + { + return internalType != tUninitialized; + } + inline void mkInt(NixInt n) { finishValue(tInt, { .integer = n }); diff --git a/tests/unit/libexpr/nix_api_value.cc b/tests/unit/libexpr/nix_api_value.cc index ac0cdb9c4..6e1131e10 100644 --- a/tests/unit/libexpr/nix_api_value.cc +++ b/tests/unit/libexpr/nix_api_value.cc @@ -14,11 +14,16 @@ namespace nixC { -TEST_F(nix_api_expr_test, nix_value_set_get_int) +TEST_F(nix_api_expr_test, nix_value_get_int_invalid) { ASSERT_EQ(0, nix_get_int(ctx, nullptr)); - ASSERT_DEATH(nix_get_int(ctx, value), ""); + assert_ctx_err(); + ASSERT_EQ(0, nix_get_int(ctx, value)); + assert_ctx_err(); +} +TEST_F(nix_api_expr_test, nix_value_set_get_int) +{ int myInt = 1; nix_init_int(ctx, value, myInt); @@ -27,24 +32,34 @@ TEST_F(nix_api_expr_test, nix_value_set_get_int) ASSERT_EQ(NIX_TYPE_INT, nix_get_type(ctx, value)); } +TEST_F(nix_api_expr_test, nix_value_set_get_float_invalid) +{ + ASSERT_DOUBLE_EQ(0.0, nix_get_float(ctx, nullptr)); + assert_ctx_err(); + ASSERT_DOUBLE_EQ(0.0, nix_get_float(ctx, value)); + assert_ctx_err(); +} + TEST_F(nix_api_expr_test, nix_value_set_get_float) { - ASSERT_FLOAT_EQ(0.0, nix_get_float(ctx, nullptr)); - ASSERT_DEATH(nix_get_float(ctx, value), ""); - - float myDouble = 1.0; + double myDouble = 1.0; nix_init_float(ctx, value, myDouble); - ASSERT_FLOAT_EQ(myDouble, nix_get_float(ctx, value)); + ASSERT_DOUBLE_EQ(myDouble, nix_get_float(ctx, value)); ASSERT_STREQ("a float", nix_get_typename(ctx, value)); ASSERT_EQ(NIX_TYPE_FLOAT, nix_get_type(ctx, value)); } -TEST_F(nix_api_expr_test, nix_value_set_get_bool) +TEST_F(nix_api_expr_test, nix_value_set_get_bool_invalid) { ASSERT_EQ(false, nix_get_bool(ctx, nullptr)); - ASSERT_DEATH(nix_get_bool(ctx, value), ""); + assert_ctx_err(); + ASSERT_EQ(false, nix_get_bool(ctx, value)); + assert_ctx_err(); +} +TEST_F(nix_api_expr_test, nix_value_set_get_bool) +{ bool myBool = true; nix_init_bool(ctx, value, myBool); @@ -53,12 +68,18 @@ TEST_F(nix_api_expr_test, nix_value_set_get_bool) ASSERT_EQ(NIX_TYPE_BOOL, nix_get_type(ctx, value)); } -TEST_F(nix_api_expr_test, nix_value_set_get_string) +TEST_F(nix_api_expr_test, nix_value_set_get_string_invalid) { std::string string_value; ASSERT_EQ(NIX_ERR_UNKNOWN, nix_get_string(ctx, nullptr, OBSERVE_STRING(string_value))); - ASSERT_DEATH(nix_get_string(ctx, value, OBSERVE_STRING(string_value)), ""); + assert_ctx_err(); + ASSERT_EQ(NIX_ERR_UNKNOWN, nix_get_string(ctx, value, OBSERVE_STRING(string_value))); + assert_ctx_err(); +} +TEST_F(nix_api_expr_test, nix_value_set_get_string) +{ + std::string string_value; const char * myString = "some string"; nix_init_string(ctx, value, myString); @@ -68,21 +89,29 @@ TEST_F(nix_api_expr_test, nix_value_set_get_string) ASSERT_EQ(NIX_TYPE_STRING, nix_get_type(ctx, value)); } +TEST_F(nix_api_expr_test, nix_value_set_get_null_invalid) +{ + ASSERT_EQ(NULL, nix_get_typename(ctx, value)); + assert_ctx_err(); +} + TEST_F(nix_api_expr_test, nix_value_set_get_null) { - ASSERT_DEATH(nix_get_typename(ctx, value), ""); - nix_init_null(ctx, value); ASSERT_STREQ("null", nix_get_typename(ctx, value)); ASSERT_EQ(NIX_TYPE_NULL, nix_get_type(ctx, value)); } -TEST_F(nix_api_expr_test, nix_value_set_get_path) +TEST_F(nix_api_expr_test, nix_value_set_get_path_invalid) { ASSERT_EQ(nullptr, nix_get_path_string(ctx, nullptr)); - ASSERT_DEATH(nix_get_path_string(ctx, value), ""); - + assert_ctx_err(); + ASSERT_EQ(nullptr, nix_get_path_string(ctx, value)); + assert_ctx_err(); +} +TEST_F(nix_api_expr_test, nix_value_set_get_path) +{ const char * p = "/nix/store/40s0qmrfb45vlh6610rk29ym318dswdr-myname"; nix_init_path_string(ctx, state, value, p); @@ -91,25 +120,39 @@ TEST_F(nix_api_expr_test, nix_value_set_get_path) ASSERT_EQ(NIX_TYPE_PATH, nix_get_type(ctx, value)); } -TEST_F(nix_api_expr_test, nix_build_and_init_list) +TEST_F(nix_api_expr_test, nix_build_and_init_list_invalid) { ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, nullptr, state, 0)); + assert_ctx_err(); ASSERT_EQ(0, nix_get_list_size(ctx, nullptr)); + assert_ctx_err(); - ASSERT_DEATH(nix_get_list_byidx(ctx, value, state, 0), ""); - ASSERT_DEATH(nix_get_list_size(ctx, value), ""); + ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, 0)); + assert_ctx_err(); + ASSERT_EQ(0, nix_get_list_size(ctx, value)); + assert_ctx_err(); +} +TEST_F(nix_api_expr_test, nix_build_and_init_list) +{ int size = 10; ListBuilder * builder = nix_make_list_builder(ctx, state, size); Value * intValue = nix_alloc_value(ctx, state); + Value * intValue2 = nix_alloc_value(ctx, state); + + // `init` and `insert` can be called in any order nix_init_int(ctx, intValue, 42); nix_list_builder_insert(ctx, builder, 0, intValue); + nix_list_builder_insert(ctx, builder, 1, intValue2); + nix_init_int(ctx, intValue2, 43); + nix_make_list(ctx, builder, value); nix_list_builder_free(builder); ASSERT_EQ(42, nix_get_int(ctx, nix_get_list_byidx(ctx, value, state, 0))); - ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, 1)); + ASSERT_EQ(43, nix_get_int(ctx, nix_get_list_byidx(ctx, value, state, 1))); + ASSERT_EQ(nullptr, nix_get_list_byidx(ctx, value, state, 2)); ASSERT_EQ(10, nix_get_list_size(ctx, value)); ASSERT_STREQ("a list", nix_get_typename(ctx, value)); @@ -119,20 +162,33 @@ TEST_F(nix_api_expr_test, nix_build_and_init_list) nix_gc_decref(ctx, intValue); } -TEST_F(nix_api_expr_test, nix_build_and_init_attr) +TEST_F(nix_api_expr_test, nix_build_and_init_attr_invalid) { ASSERT_EQ(nullptr, nix_get_attr_byname(ctx, nullptr, state, 0)); + assert_ctx_err(); ASSERT_EQ(nullptr, nix_get_attr_byidx(ctx, nullptr, state, 0, nullptr)); + assert_ctx_err(); ASSERT_EQ(nullptr, nix_get_attr_name_byidx(ctx, nullptr, state, 0)); + assert_ctx_err(); ASSERT_EQ(0, nix_get_attrs_size(ctx, nullptr)); + assert_ctx_err(); ASSERT_EQ(false, nix_has_attr_byname(ctx, nullptr, state, "no-value")); + assert_ctx_err(); - ASSERT_DEATH(nix_get_attr_byname(ctx, value, state, 0), ""); - ASSERT_DEATH(nix_get_attr_byidx(ctx, value, state, 0, nullptr), ""); - ASSERT_DEATH(nix_get_attr_name_byidx(ctx, value, state, 0), ""); - ASSERT_DEATH(nix_get_attrs_size(ctx, value), ""); - ASSERT_DEATH(nix_has_attr_byname(ctx, value, state, "no-value"), ""); + ASSERT_EQ(nullptr, nix_get_attr_byname(ctx, value, state, 0)); + assert_ctx_err(); + ASSERT_EQ(nullptr, nix_get_attr_byidx(ctx, value, state, 0, nullptr)); + assert_ctx_err(); + ASSERT_EQ(nullptr, nix_get_attr_name_byidx(ctx, value, state, 0)); + assert_ctx_err(); + ASSERT_EQ(0, nix_get_attrs_size(ctx, value)); + assert_ctx_err(); + ASSERT_EQ(false, nix_has_attr_byname(ctx, value, state, "no-value")); + assert_ctx_err(); +} +TEST_F(nix_api_expr_test, nix_build_and_init_attr) +{ int size = 10; const char ** out_name = (const char **) malloc(sizeof(char *)); @@ -311,4 +367,17 @@ TEST_F(nix_api_expr_test, nix_value_init_apply_lazy_arg) nix_gc_decref(ctx, e); } +TEST_F(nix_api_expr_test, nix_copy_value) +{ + Value * source = nix_alloc_value(ctx, state); + + nix_init_int(ctx, source, 42); + nix_copy_value(ctx, value, source); + + ASSERT_EQ(42, nix_get_int(ctx, value)); + + // Clean up + nix_gc_decref(ctx, source); +} + } diff --git a/tests/unit/libexpr/value/value.cc b/tests/unit/libexpr/value/value.cc new file mode 100644 index 000000000..5762d5891 --- /dev/null +++ b/tests/unit/libexpr/value/value.cc @@ -0,0 +1,25 @@ +#include "value.hh" + +#include "tests/libstore.hh" + +namespace nix { + +class ValueTest : public LibStoreTest +{}; + +TEST_F(ValueTest, unsetValue) +{ + Value unsetValue; + ASSERT_EQ(false, unsetValue.isValid()); + ASSERT_EQ(nThunk, unsetValue.type(true)); + ASSERT_DEATH(unsetValue.type(), ""); +} + +TEST_F(ValueTest, vInt) +{ + Value vInt; + vInt.mkInt(42); + ASSERT_EQ(true, vInt.isValid()); +} + +} // namespace nix diff --git a/tests/unit/libutil-support/tests/nix_api_util.hh b/tests/unit/libutil-support/tests/nix_api_util.hh index 75d302bd6..efd200116 100644 --- a/tests/unit/libutil-support/tests/nix_api_util.hh +++ b/tests/unit/libutil-support/tests/nix_api_util.hh @@ -24,7 +24,9 @@ protected: nix_c_context * ctx; - inline void assert_ctx_ok() { + inline void assert_ctx_ok() + { + if (nix_err_code(ctx) == NIX_OK) { return; } @@ -33,5 +35,14 @@ protected: std::string msg(p, n); FAIL() << "nix_err_code(ctx) != NIX_OK, message: " << msg; } + + inline void assert_ctx_err() + { + if (nix_err_code(ctx) != NIX_OK) { + return; + } + FAIL() << "Got NIX_OK, but expected an error!"; + } }; + }