From 123c0c1bc8d537382b17c77af5b2909edf2e485f Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Fri, 27 Jun 2025 18:56:32 +0300 Subject: mu-scm: defer scm exceptions with c++ throw/catch SCM exception are "non-local exits", which mean that c++ objects don't get their DTOR called when leaving the scope.... this break RAII, leaks mem etc. So instead, we avoid SCM exceptions (where we can), and throw C++ exceptions, and only use SCM-expections in the catch block. --- scm/mu-scm-message.cc | 42 ++++++++++---------- scm/mu-scm-store.cc | 69 ++++++++++++++++---------------- scm/mu-scm.hh | 106 +++++++++++++++++++++++++++++++++++++------------- 3 files changed, 136 insertions(+), 81 deletions(-) diff --git a/scm/mu-scm-message.cc b/scm/mu-scm-message.cc index 902608f..2cdee34 100644 --- a/scm/mu-scm-message.cc +++ b/scm/mu-scm-message.cc @@ -41,9 +41,11 @@ static MessageMap message_map; } static const Message& -to_message(SCM scm) +to_message(SCM scm, const char *func, int pos) { - scm_assert_foreign_object_type(message_type, scm); + if (!SCM_IS_A_P(scm, message_type)) + throw ScmError{ScmError::Id::WrongType, func, pos, scm, "mesagestore"}; + return *reinterpret_cast(scm_foreign_object_ref(scm, 0)); } @@ -58,8 +60,7 @@ finalize_message(SCM scm) } static SCM -subr_message_object_make(SCM message_path_scm) -{ +subr_message_object_make(SCM message_path_scm) try { // message objects eat fds, tickle the gc... letting it handle it // automatically is not soon enough. if (message_map.size() >= 0.8 * max_message_map_size) @@ -70,48 +71,49 @@ subr_message_object_make(SCM message_path_scm) // qttempt to give an good error message rather then getting something // from GMime) if (message_map.size() >= max_message_map_size) - raise_error("failure", "message-object", - "too many open messages"); + throw ScmError{"make-message", "too many open messages"}; // if we already have the message in our map, return it. - auto path{from_scm(message_path_scm)}; + auto path{from_scm(message_path_scm, "make-message", 1)}; if (const auto it = message_map.find(path); it != message_map.end()) return it->second.foreign_object; // don't have it yet; attempt to create one - if (auto res{Message::make_from_path(path)}; !res) { - raise_error("failure", "message-object", - "failed to create message from {}: {}", path, res.error()); - return SCM_BOOL_F; - } else { + if (auto res{Message::make_from_path(path)}; !res) + throw ScmError{"make-message", "failed to create message"}; + else { // create a new object, store it in our map and return the foreign ptr. auto it = message_map.emplace(std::move(path), std::move(*res)); return it.first->second.foreign_object = scm_make_foreign_object_1( message_type, const_cast(&it.first->second.message)); } +} catch (const ScmError& err) { + err.throw_scm(); } static SCM -subr_message_body(SCM message_scm, SCM html_scm) -{ - const auto& message{to_message(message_scm)}; - const auto html{from_scm(html_scm)}; +subr_message_body(SCM message_scm, SCM html_scm) try { + const auto& message{to_message(message_scm, "body", 1)}; + const auto html{from_scm(html_scm, "message-body", 2)}; if (const auto body{html ? message.body_html() : message.body_text()}; body) return to_scm(*body); else return SCM_BOOL_F; +} catch (const ScmError& err) { + err.throw_scm(); } static SCM -subr_message_header(SCM message_scm, SCM field_scm) -{ - const auto& message{to_message(message_scm)}; - const auto field{from_scm(field_scm)}; +subr_message_header(SCM message_scm, SCM field_scm) try { + const auto& message{to_message(message_scm, "header", 1)}; + const auto field{from_scm(field_scm, "message-header", 2)}; if (const auto val{message.header(field)}; val) return to_scm(*val); else return SCM_BOOL_F; +} catch (const ScmError& err) { + err.throw_scm(); } static void diff --git a/scm/mu-scm-store.cc b/scm/mu-scm-store.cc index 32ee41b..05cdb91 100644 --- a/scm/mu-scm-store.cc +++ b/scm/mu-scm-store.cc @@ -30,71 +30,72 @@ static bool initialized; } static const Store& -to_store(SCM scm) +to_store(SCM scm, const char *func, int pos) { - scm_assert_foreign_object_type(store_type, scm); + if (!SCM_IS_A_P(scm, store_type)) + throw ScmError{ScmError::Id::WrongType, func, pos, scm, "store"}; + return *reinterpret_cast(scm_foreign_object_ref(scm, 0)); } static SCM -subr_mcount(SCM store_scm) -{ - return to_scm(to_store(store_scm).size()); +subr_mcount(SCM store_scm) try { + return to_scm(to_store(store_scm, "mcount", 1).size()); +} catch (const ScmError& err) { + err.throw_scm(); } static SCM -subr_cfind(SCM store_scm, SCM pattern_scm, SCM personal_scm, SCM after_scm, SCM max_results_scm) -{ - SCM contacts{SCM_EOL}; - const auto pattern{from_scm(pattern_scm)}; - const auto personal{from_scm(personal_scm)}; - const auto after{from_scm_with_default(after_scm, 0)}; +subr_cfind(SCM store_scm, SCM pattern_scm, SCM personal_scm, SCM after_scm, SCM max_results_scm) try { + SCM contacts{SCM_EOL}; + const auto pattern{from_scm(pattern_scm, "cfind", 2)}; + const auto personal{from_scm(personal_scm, "cfind", 3)}; + const auto after{from_scm_with_default(after_scm, 0, "cfind", 4)}; // 0 means "unlimited" - const size_t maxnum = from_scm_with_default(max_results_scm, 0U); + const size_t maxnum = from_scm_with_default(max_results_scm, 0U, "cfind", 5); - to_store(store_scm).contacts_cache().for_each( + to_store(store_scm, "cfind", 1).contacts_cache().for_each( [&](const auto& contact)->bool { contacts = scm_append_x(scm_list_2(contacts, scm_list_1(to_scm(contact)))); return true; }, pattern, personal, after, maxnum); + return contacts; +} catch (const ScmError& scm_err) { + scm_err.throw_scm(); } static Field::Id -to_sort_field_id(SCM field) +to_sort_field_id(SCM field, const char *func, int pos) { if (scm_is_false(field)) return Field::Id::Date; - const auto sym{from_scm(scm_symbol_to_string(field))}; + const auto sym{from_scm(scm_symbol_to_string(field), func, pos)}; if (const auto field_opt{field_from_name(sym)}; !field_opt) { - raise_error("invalid symbol", "mfind", - "expected sort-field symbol, but got {}", sym); - return Field::Id::Date; + throw ScmError{ScmError::Id::WrongType, func, pos, field, "sort-field-symbol"}; } else return field_opt->id; } static SCM subr_mfind(SCM store_scm, SCM query_scm, SCM related_scm, SCM skip_dups_scm, - SCM sort_field_scm, SCM reverse_scm, SCM max_results_scm) -{ - const auto& store{to_store(store_scm)}; - const auto query{from_scm(query_scm)}; - const auto related(from_scm(related_scm)); - const auto skip_dups(from_scm(skip_dups_scm)); - const auto reverse(from_scm(reverse_scm)); + SCM sort_field_scm, SCM reverse_scm, SCM max_results_scm) try { - SCM_ASSERT_TYPE(scm_is_false(sort_field_scm) || scm_is_symbol(sort_field_scm), - sort_field_scm, SCM_ARG5, __func__, "symbol or #f"); + const auto& store{to_store(store_scm, "mfind", 1)}; + const auto query{from_scm(query_scm, "mfind", 2)}; + const auto related(from_scm(related_scm, "mfind", 3)); + const auto skip_dups(from_scm(skip_dups_scm, "mfind", 4)); - const auto sort_field_id = to_sort_field_id(sort_field_scm); + if (!scm_is_false(sort_field_scm) && !scm_is_symbol(sort_field_scm)) + throw ScmError{ScmError::Id::WrongType, "mfind", 5, sort_field_scm, "#f or sort-field-symbol"}; - // 0 means "unlimited" - const size_t maxnum = from_scm_with_default(max_results_scm, 0U); + const auto sort_field_id = to_sort_field_id(sort_field_scm, "mfind", 5); + const auto reverse(from_scm(reverse_scm, "mfind", 6)); - // XXX date/reverse/maxnum + // 0 means "unlimited" + const size_t maxnum = from_scm_with_default(max_results_scm, 0U, "mfind", 7); const QueryFlags qflags = QueryFlags::SkipUnreadable | (skip_dups ? QueryFlags::SkipDuplicates : QueryFlags::None) | @@ -105,7 +106,8 @@ subr_mfind(SCM store_scm, SCM query_scm, SCM related_scm, SCM skip_dups_scm, std::lock_guard lock{store.lock()}; const auto qres = store.run_query(query, sort_field_id, qflags, maxnum); - SCM_ASSERT(qres, query_scm, SCM_ARG1, __func__); + if (!qres) + throw ScmError{ScmError::Id::WrongArg, "mfind", 2, query_scm, ""}; for (const auto& mi: *qres) { if (auto plist{mi.document()->get_data()}; plist.empty()) @@ -117,8 +119,9 @@ subr_mfind(SCM store_scm, SCM query_scm, SCM related_scm, SCM skip_dups_scm, } return msgs; +} catch (const ScmError& err) { + err.throw_scm(); } - static void init_subrs() { diff --git a/scm/mu-scm.hh b/scm/mu-scm.hh index cbb2a15..e16760c 100644 --- a/scm/mu-scm.hh +++ b/scm/mu-scm.hh @@ -74,6 +74,55 @@ namespace Mu::Scm { std::is_array_v && std::is_same_v, char>; + + /** + * C++ Exception to capture an SCM exception + * + * An ScmError is a C++ exception that captures the information for a + * Scm exception to be thrown later at some convenient moment (_after_ + * an orderly finishing of the block, so DTORs etc can run.) + */ + struct ScmError { + enum struct Id { WrongType, WrongArg, Error }; + ScmError(Id id, const char *subr, int pos, SCM bad_val, + const char *expected): + id{Id::WrongType}, subr{subr}, pos(pos), bad_val{bad_val}, + expected{expected} {} + ScmError(const char *subr, const char *msg): + id{Id::Error}, subr{subr}, msg{msg} {} + + /** + * This will do a "non-local exit", ie. does not return. + * + */ + [[noreturn]] void throw_scm() const { + /* Enforce exhaustive switch (do _not_ add a default case) */ +#pragma GCC diagnostic push +#pragma GCC diagnostic error "-Wswitch" + switch(id) { + case Id::WrongType: + case Id::WrongArg: + scm_wrong_type_arg_msg(subr, pos, bad_val, expected); + break; + case Id::Error: + scm_misc_error(subr, msg, SCM_BOOL_F); + break; +#pragma GCC diagnostic ignored "-Wswitch-default" +#pragma GCC diagnostic pop + } + throw; // never reached, just to appease compiler. + } + + private: + const Id id; + const char *subr{}; + int pos; + SCM bad_val; + const char *expected; + const char *msg{}; + }; + + /** * Make SCM symbol from string-like value * @@ -98,34 +147,43 @@ namespace Mu::Scm { /** * Get some C++ value from an SCM object, generically. * + * This throws ScmError in case of any errors. + * * @param ARG some SCM object + * @param func function where this is used + * @param pos argument number (starting from 1) + * @param expected the expected type * * @return C++ value */ template - T from_scm(SCM ARG) { + T from_scm(SCM ARG, const char *func, int pos) { + const auto ensure=[&](bool pred, SCM ARG, const char *expected) { + if (!pred) + throw ScmError{ScmError::Id::WrongType, func, pos, ARG, expected}; + }; using Type = std::remove_const_t; // *not* std::remove_const if constexpr (std::is_same_v) { - SCM_ASSERT(scm_string_p(ARG), ARG, SCM_ARG1, __func__); + ensure(scm_string_p(ARG), ARG, "string"); auto str{scm_to_utf8_string(ARG)}; std::string res{str}; ::free(str); return res; } else if constexpr (std::is_same_v) { - SCM_ASSERT(scm_char_p(ARG), ARG, SCM_ARG1, __func__); + ensure(scm_char_p(ARG), ARG, "character"); return scm_to_char(ARG); } else if constexpr (std::is_same_v) { - SCM_ASSERT(scm_boolean_p(ARG), ARG, SCM_ARG1, __func__); + ensure(scm_boolean_p(ARG), ARG, "bool"); return scm_to_bool(ARG); } else if constexpr (std::is_same_v) { - SCM_ASSERT(scm_is_signed_integer(ARG, std::numeric_limits::min(), + ensure(scm_is_signed_integer(ARG, std::numeric_limits::min(), std::numeric_limits::max()), - ARG, SCM_ARG1, __func__); + ARG, "integer"); return scm_to_int(ARG); } else if constexpr (std::is_same_v) { - SCM_ASSERT(scm_is_unsigned_integer(ARG, std::numeric_limits::min(), + ensure(scm_is_unsigned_integer(ARG, std::numeric_limits::min(), std::numeric_limits::max()), - ARG, SCM_ARG1, __func__); + ARG, "unsigned"); return scm_to_uint(ARG); } else if constexpr (std::is_same_v) { return ARG; @@ -143,8 +201,8 @@ namespace Mu::Scm { * @return value */ template - T from_scm_with_default(SCM ARG, const T default_value) { - return (scm_is_bool(ARG) && scm_is_false(ARG)) ? default_value : from_scm(ARG); + T from_scm_with_default(SCM ARG, const T default_value, const char *func, int pos) { + return (scm_is_bool(ARG) && scm_is_false(ARG)) ? default_value : from_scm(ARG, func, pos); } @@ -203,26 +261,18 @@ namespace Mu::Scm { return alist_add(res, std::forward(keyvals)...); } - /** - * Make an SCM error - * - * @param err name of the error - * @param subr function name - * @param frm... args format string - * - * @return an error (type) - */ - template - void raise_error(const std::string& err, - const std::string& subr, - fmt::format_string frm, T&&... args) noexcept { - static SCM mu_scm_error = scm_from_utf8_symbol("mu-scm-error"); - scm_error(mu_scm_error, - subr.c_str(), - fmt::format(frm, std::forward(args)...).c_str(), - SCM_BOOL_F, SCM_BOOL_F); + //template + static inline SCM try_scm(scm_t_catch_body func, scm_t_catch_handler handler) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wcast-function-type" + return scm_internal_catch( + SCM_BOOL_F, + func, SCM_BOOL_F, + handler, SCM_BOOL_F); +#pragma GCC diagnostic pop } + /**@}*/ } -- cgit v1.0