From c6a52ede23f690bea260b3b653eff241f6fceb99 Mon Sep 17 00:00:00 2001 From: Eduardo Bart Date: Tue, 8 Oct 2024 09:30:14 -0300 Subject: [PATCH] refactor!: return cm_error for cm_get_default_config/cm_get_reg_address --- src/clua-machine.cpp | 10 ++++-- src/machine-c-api.cpp | 32 ++++++++++++------- src/machine-c-api.h | 23 ++++++++------ tests/misc/test-machine-c-api.cpp | 52 +++++++++++++++++++------------ 4 files changed, 72 insertions(+), 45 deletions(-) diff --git a/src/clua-machine.cpp b/src/clua-machine.cpp index e0dbc0874..e67f75b3e 100644 --- a/src/clua-machine.cpp +++ b/src/clua-machine.cpp @@ -25,8 +25,8 @@ namespace cartesi { /// \brief This is the machine.get_default_machine_config() /// method implementation. static int machine_class_index_get_default_config(lua_State *L) { - const char *config = cm_get_default_config(); - if (!config) { + const char *config{}; + if (cm_get_default_config(&config) != 0) { return luaL_error(L, "%s", cm_get_last_error_message()); } clua_push_json_table(L, config); @@ -35,7 +35,11 @@ static int machine_class_index_get_default_config(lua_State *L) { /// \brief This is the machine.get_reg_address() method implementation. static int machine_class_index_get_reg_address(lua_State *L) { - lua_pushinteger(L, static_cast(cm_get_reg_address(clua_check_cm_proc_reg(L, 1)))); + uint64_t addr{}; + if (cm_get_reg_address(clua_check_cm_proc_reg(L, 1), &addr) != 0) { + return luaL_error(L, "%s", cm_get_last_error_message()); + } + lua_pushinteger(L, static_cast(addr)); return 1; } diff --git a/src/machine-c-api.cpp b/src/machine-c-api.cpp index 755fbc5fe..ac2750779 100644 --- a/src/machine-c-api.cpp +++ b/src/machine-c-api.cpp @@ -384,14 +384,18 @@ cm_error cm_write_reg(cm_machine *m, cm_reg reg, uint64_t val) try { return cm_result_failure(); } -uint64_t cm_get_reg_address(cm_reg reg) try { +cm_error cm_get_reg_address(cm_reg reg, uint64_t *val) try { + if (val == nullptr) { + throw std::invalid_argument("invalid val output"); + } auto cpp_reg = static_cast(reg); - uint64_t address = cartesi::machine::get_reg_address(cpp_reg); - cm_result_success(); - return address; + *val = cartesi::machine::get_reg_address(cpp_reg); + return cm_result_success(); } catch (...) { - cm_result_failure(); - return UINT64_MAX; + if (val) { + *val = 0; + } + return cm_result_failure(); } cm_error cm_read_word(const cm_machine *m, uint64_t address, uint64_t *val) try { @@ -568,16 +572,20 @@ cm_error cm_get_initial_config(const cm_machine *m, const char **config) try { return cm_result_failure(); } -const char *cm_get_default_config() try { +cm_error cm_get_default_config(const char **config) try { + if (config == nullptr) { + throw std::invalid_argument("invalid config output"); + } const cartesi::machine_config cpp_config = cartesi::machine::get_default_config(); static THREAD_LOCAL std::string config_storage; config_storage = cartesi::to_json(cpp_config).dump(); - const char *config = config_storage.c_str(); - cm_result_success(); - return config; + *config = config_storage.c_str(); + return cm_result_success(); } catch (...) { - cm_result_failure(); - return nullptr; + if (config) { + *config = nullptr; + } + return cm_result_failure(); } cm_error cm_replace_memory_range(cm_machine *m, uint64_t start, uint64_t length, bool shared, diff --git a/src/machine-c-api.h b/src/machine-c-api.h index 6b6c61bc7..89903d013 100644 --- a/src/machine-c-api.h +++ b/src/machine-c-api.h @@ -312,20 +312,23 @@ typedef struct cm_machine cm_machine; /// (Instead, use the return code of the previous call itself.) CM_API const char *cm_get_last_error_message(); -/// \brief Returns a JSON object with the default machine config as a string. -/// \returns A C string, in case of success, guaranteed to remain valid only until the -/// the next time this same function is called again on the same thread. -/// In case of failure, NULL is returned and last error message is set. +/// \brief Obtains a JSON object with the default machine config as a string. +/// \param config Receives the default configuration as a JSON object in a string, +/// guaranteed to remain valid only until the the next time this same function +/// is called again on the same thread. +/// \returns 0 for success, non zero code for error. /// \details The returned config is not sufficient to run a machine. /// Additional configurations, such as RAM length, RAM image, flash drives, /// and entrypoint are still needed. -CM_API const char *cm_get_default_config(); +CM_API cm_error cm_get_default_config(const char **config); /// \brief Gets the address of any x, f, or control state register. /// \param reg The register. -/// \returns The address of the specified register. -/// In case the register is invalid, UINT64_MAX is returned and last error message is set. -CM_API uint64_t cm_get_reg_address(cm_reg reg); +/// \param val Receives address of the register. +/// \returns 0 for success, non zero code for error. +/// \details The current implementation of this function is slow when the word falls +/// in a memory range mapped to a device. +CM_API cm_error cm_get_reg_address(cm_reg reg, uint64_t *val); // ----------------------------------------------------------------------------- // Machine API functions @@ -376,7 +379,7 @@ CM_API cm_error cm_store(const cm_machine *m, const char *dir); CM_API cm_error cm_replace_memory_range(cm_machine *m, uint64_t start, uint64_t length, bool shared, const char *image_filename); -/// \brief Returns a JSON object with the machine config used to initialize the machine. +/// \brief Obtains a JSON object with the machine config used to initialize the machine. /// \param m Pointer to a valid machine instance. /// \param config Receives the initial configuration as a JSON object in a /// string, guaranteed to remain valid only until the the next time this same function @@ -384,7 +387,7 @@ CM_API cm_error cm_replace_memory_range(cm_machine *m, uint64_t start, uint64_t /// \returns 0 for success, non zero code for error. CM_API cm_error cm_get_initial_config(const cm_machine *m, const char **config); -/// \brief Returns a list with all memory ranges in the machine. +/// \brief Obtains a list with all memory ranges in the machine. /// \param m Pointer to a valid machine instance. /// \param ranges Receives the memory ranges as a JSON object in a string, /// guaranteed to remain valid only until the the next time this same function is diff --git a/tests/misc/test-machine-c-api.cpp b/tests/misc/test-machine-c-api.cpp index 13dd624b8..468650066 100644 --- a/tests/misc/test-machine-c-api.cpp +++ b/tests/misc/test-machine-c-api.cpp @@ -59,15 +59,19 @@ BOOST_AUTO_TEST_CASE_NOLINT(delete_machine_null_test) { } BOOST_AUTO_TEST_CASE_NOLINT(get_default_machine_config_basic_test) { - const char *config = cm_get_default_config(); - BOOST_TEST_CHECK(config != nullptr); + const char *config{}; + cm_error error_code = cm_get_default_config(&config); + BOOST_CHECK_EQUAL(error_code, CM_ERROR_OK); BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); + BOOST_TEST_CHECK(config != nullptr); } class default_machine_fixture { public: default_machine_fixture() { - _default_machine_config = cm_get_default_config(); + const char *config{}; + cm_get_default_config(&config); + _default_machine_config = config; } ~default_machine_fixture() {} @@ -79,7 +83,7 @@ class default_machine_fixture { protected: cm_machine *_machine{}; - const char *_default_machine_config{}; + std::string _default_machine_config{}; }; BOOST_FIXTURE_TEST_CASE_NOLINT(load_machine_unknown_dir_test, default_machine_fixture) { @@ -106,14 +110,8 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(create_machine_null_config_test, default_machine_ BOOST_CHECK_EQUAL(origin, result); } -BOOST_FIXTURE_TEST_CASE_NOLINT(create_machine_null_rt_config_test, default_machine_fixture) { - cm_error error_code = cm_create(_default_machine_config, nullptr, &_machine); - BOOST_CHECK_EQUAL(error_code, CM_ERROR_INVALID_ARGUMENT); - BOOST_REQUIRE_EQUAL(std::string(cm_get_last_error_message()), std::string("RAM length cannot be zero")); -} - BOOST_FIXTURE_TEST_CASE_NOLINT(create_machine_default_machine_test, default_machine_fixture) { - cm_error error_code = cm_create(_default_machine_config, nullptr, &_machine); + cm_error error_code = cm_create(_default_machine_config.c_str(), nullptr, &_machine); BOOST_CHECK_EQUAL(error_code, CM_ERROR_INVALID_ARGUMENT); BOOST_REQUIRE_EQUAL(std::string(cm_get_last_error_message()), std::string("RAM length cannot be zero")); } @@ -122,7 +120,9 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(create_machine_default_machine_test, default_mach class incomplete_machine_fixture : public default_machine_fixture { public: incomplete_machine_fixture() : _machine_config{} { - _machine_config = nlohmann::json::parse(cm_get_default_config()); + const char *config{}; + cm_get_default_config(&config); + _machine_config = nlohmann::json::parse(config); _machine_config["ram"]["length"] = 1 << 20; } @@ -223,7 +223,7 @@ class serialized_machine_fixture : public ordinary_machine_fixture { BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); } - virtual ~serialized_machine_fixture() { + ~serialized_machine_fixture() { std::filesystem::remove_all(_machine_config_path.string()); } @@ -260,9 +260,12 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(load_machine_invalid_config_version_test, seriali std::stringstream expected_err; expected_err << "expected \"archive_version\" " << v << " (got " << v + 1 << ")"; - cm_error error_code = cm_load(_machine_config_path.c_str(), nullptr, &_machine); + cm_machine *restored_machine{}; + cm_error error_code = cm_load(_machine_config_path.c_str(), nullptr, &restored_machine); BOOST_CHECK_EQUAL(error_code, CM_ERROR_RUNTIME_ERROR); BOOST_CHECK_EQUAL(std::string(cm_get_last_error_message()), expected_err.str()); + + cm_destroy(restored_machine, false); } class store_file_fixture : public ordinary_machine_fixture { @@ -471,7 +474,9 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(read_word_null_output_test, default_machine_fixtu BOOST_FIXTURE_TEST_CASE_NOLINT(read_word_basic_test, ordinary_machine_fixture) { uint64_t word_value = 0; - cm_error error_code = cm_read_word(_machine, cm_get_reg_address(CM_REG_PC), &word_value); + uint64_t pc_addr{}; + BOOST_CHECK_EQUAL(cm_get_reg_address(CM_REG_PC, &pc_addr), CM_ERROR_OK); + cm_error error_code = cm_read_word(_machine, pc_addr, &word_value); BOOST_CHECK_EQUAL(error_code, CM_ERROR_OK); BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); BOOST_CHECK_EQUAL(word_value, static_cast(0x80000000)); @@ -1038,7 +1043,9 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(read_write_x_basic_test, ordinary_machine_fixture BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); BOOST_CHECK_EQUAL(x_origin, x_read); - BOOST_CHECK_EQUAL(static_cast(0x10), cm_get_reg_address(CM_REG_X2)); + uint64_t x2_addr{}; + BOOST_CHECK_EQUAL(cm_get_reg_address(CM_REG_X2, &x2_addr), CM_ERROR_OK); + BOOST_CHECK_EQUAL(static_cast(0x10), x2_addr); } BOOST_FIXTURE_TEST_CASE_NOLINT(read_write_f_basic_test, ordinary_machine_fixture) { @@ -1053,7 +1060,9 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(read_write_f_basic_test, ordinary_machine_fixture BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); BOOST_CHECK_EQUAL(f_origin, f_read); - BOOST_CHECK_EQUAL(static_cast(0x110), cm_get_reg_address(CM_REG_F2)); + uint64_t f2_addr{}; + BOOST_CHECK_EQUAL(cm_get_reg_address(CM_REG_F2, &f2_addr), CM_ERROR_OK); + BOOST_CHECK_EQUAL(static_cast(0x110), f2_addr); } BOOST_FIXTURE_TEST_CASE_NOLINT(read_write_uarch_x_basic_test, ordinary_machine_fixture) { @@ -1068,8 +1077,9 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(read_write_uarch_x_basic_test, ordinary_machine_f BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); BOOST_CHECK_EQUAL(uarch_x_origin, uarch_x_read); - BOOST_CHECK_EQUAL(static_cast(cartesi::PMA_SHADOW_UARCH_STATE_START + 40), - cm_get_reg_address(CM_REG_UARCH_X2)); + uint64_t uarch_x2_addr{}; + BOOST_CHECK_EQUAL(cm_get_reg_address(CM_REG_UARCH_X2, &uarch_x2_addr), CM_ERROR_OK); + BOOST_CHECK_EQUAL(static_cast(cartesi::PMA_SHADOW_UARCH_STATE_START + 40), uarch_x2_addr); } BOOST_AUTO_TEST_CASE_NOLINT(read_reg_null_machine_test) { @@ -1101,7 +1111,9 @@ BOOST_FIXTURE_TEST_CASE_NOLINT(read_write_reg_basic_test, ordinary_machine_fixtu BOOST_CHECK_EQUAL(std::string(""), std::string(cm_get_last_error_message())); BOOST_CHECK_EQUAL(reg_origin, reg_read); - BOOST_CHECK_EQUAL(static_cast(0x200), cm_get_reg_address(CM_REG_PC)); + uint64_t pc_addr{}; + BOOST_CHECK_EQUAL(cm_get_reg_address(CM_REG_PC, &pc_addr), CM_ERROR_OK); + BOOST_CHECK_EQUAL(static_cast(0x200), pc_addr); } BOOST_AUTO_TEST_CASE_NOLINT(verify_merkle_tree_null_machine_test) {