scm/c++: fix review issues in handler code

- Fix comments and some debug strings (copy-pasta)
- Fix mime-stream destruction (g_object_unref)
- Fix type-checking of scm types
This commit is contained in:
Dirk-Jan C. Binnema
2026-07-21 23:31:58 +03:00
committed by Seth Ladygo
parent 2c2bc317f3
commit 60ed8f1a12
5 changed files with 38 additions and 23 deletions

View File

@ -46,7 +46,7 @@ static const Message&
to_message(SCM scm, const char *func, int pos) to_message(SCM scm, const char *func, int pos)
{ {
if (!SCM_IS_A_P(scm, message_type)) if (!SCM_IS_A_P(scm, message_type))
throw ScmError{ScmError::Id::WrongType, func, pos, scm, "mesagestore"}; throw ScmError{ScmError::Id::WrongType, func, pos, scm, "message"};
return *reinterpret_cast<Message*>(scm_foreign_object_ref(scm, 0)); return *reinterpret_cast<Message*>(scm_foreign_object_ref(scm, 0));
} }
@ -67,16 +67,19 @@ subr_cc_message_make(SCM message_path_scm) try {
constexpr auto func{"cc-message-make"}; constexpr auto func{"cc-message-make"};
// message objects eat fds, tickle the gc... letting it handle it // message objects eat fds, tickle the gc... letting it handle it
// automatically is not soon enough. // automatically is not soon enough. Note: if a script _holds_
// references to this many messages, the map cannot shrink and this
// triggers a full GC on each call; slow, but better than running
// out of fds.
if (message_map.size() >= 0.8 * max_message_map_size) if (message_map.size() >= 0.8 * max_message_map_size)
scm_gc(); scm_gc();
std::unique_lock lock{map_lock}; std::unique_lock lock{map_lock};
// qttempt to give an good error message rather then getting something // attempt to give a good error message rather than getting something
// from GMime) // from GMime)
if (message_map.size() >= max_message_map_size) if (message_map.size() >= max_message_map_size)
throw ScmError{"cc-make-message", "too many open messages"}; throw ScmError{func, "too many open messages"};
// if we already have the message in our map, return it. // if we already have the message in our map, return it.
auto path{from_scm<std::string>(message_path_scm, func, 1)}; auto path{from_scm<std::string>(message_path_scm, func, 1)};
@ -103,7 +106,7 @@ subr_cc_message_make(SCM message_path_scm) try {
static SCM static SCM
subr_cc_message_body(SCM message_scm, SCM html_scm) try { subr_cc_message_body(SCM message_scm, SCM html_scm) try {
constexpr auto func{"cc-message-make"}; constexpr auto func{"cc-message-body"};
const auto& message{to_message(message_scm, func, 1)}; const auto& message{to_message(message_scm, func, 1)};
const auto html{from_scm<bool>(html_scm, func, 2)}; const auto html{from_scm<bool>(html_scm, func, 2)};

View File

@ -80,7 +80,11 @@ make_mime_stream_port_type()
{ {
auto ptype = scm_make_port_type(const_cast<char*>("mime-stream"), mime_stream_read, {}); auto ptype = scm_make_port_type(const_cast<char*>("mime-stream"), mime_stream_read, {});
scm_set_port_close(ptype, [](SCM port){g_mime_stream_close(from_scm_port(port));}); scm_set_port_close(ptype, [](SCM port){
auto stream{from_scm_port(port)};
g_mime_stream_close(stream);
g_object_unref(stream); // the port owns the stream
});
scm_set_port_needs_close_on_gc(ptype, true); scm_set_port_needs_close_on_gc(ptype, true);
scm_set_port_seek(ptype, mime_stream_seek); scm_set_port_seek(ptype, mime_stream_seek);
@ -164,9 +168,9 @@ subr_make_mime_stream_port(SCM mime_part_scm, SCM content_only_scm,
GMimeStream *stream{}; GMimeStream *stream{};
try { try {
auto part = part_from_scm(mime_part_scm, func, 1); auto part = part_from_scm(mime_part_scm, func, 1);
const auto decode{from_scm<bool>(decode_scm,
func, 2)};
const auto content_only{from_scm<bool>(content_only_scm, const auto content_only{from_scm<bool>(content_only_scm,
func, 2)};
const auto decode{from_scm<bool>(decode_scm,
func, 3)}; func, 3)};
if (decode) if (decode)
stream = get_decoded_stream(part); stream = get_decoded_stream(part);

View File

@ -167,9 +167,13 @@ subr_cc_store_mfind(SCM store_scm, SCM query_scm, SCM related_scm, SCM skip_dups
SCM msgs{SCM_EOL}; SCM msgs{SCM_EOL};
// iterate in reverse order, so the message get consed // iterate in reverse order, so the message get consed
// into the list in the right order. // into the list in the right order.
for (auto it{qres->end()}; it-- != qres->begin();) for (auto it{qres->end()}; it-- != qres->begin();) {
if (auto plist{it.document()->get_data()}; !plist.empty()) const auto doc{it.document()};
if (!doc)
continue; // e.g., removed since the query ran
if (auto plist{doc->get_data()}; !plist.empty())
msgs = scm_cons(to_scm(plist), msgs); msgs = scm_cons(to_scm(plist), msgs);
}
return msgs; return msgs;
} catch (const ScmError& err) { } catch (const ScmError& err) {

View File

@ -160,12 +160,12 @@ maybe_remove_socket_path()
// opportunistic, so no real warnings, but be careful deleting! // opportunistic, so no real warnings, but be careful deleting!
if (const int res = ::stat(sock.c_str(), &statbuf); res != 0) { if (::stat(sock.c_str(), &statbuf) != 0) {
mu_debug("can't stat '{}'; err={}", sock, -res); mu_debug("can't stat '{}': {}", sock, ::strerror(errno));
} else if ((statbuf.st_mode & S_IFMT) != S_IFSOCK) { } else if ((statbuf.st_mode & S_IFMT) != S_IFSOCK) {
mu_debug("{} is not a socket", sock); mu_debug("{} is not a socket", sock);
} else if (const int ulres = ::unlink(sock.c_str()); ulres != 0) { } else if (::unlink(sock.c_str()) != 0) {
mu_debug("failed to unlink '{}'; err={}", sock, -ulres); mu_debug("failed to unlink '{}': {}", sock, ::strerror(errno));
} else { } else {
mu_debug("unlinked {}", sock); mu_debug("unlinked {}", sock);
} }
@ -308,7 +308,7 @@ test_scm_script()
MemDb mdb; MemDb mdb;
Config conf{mdb}; Config conf{mdb};
; conf.set<Config::Id::PersonalAddresses>( conf.set<Config::Id::PersonalAddresses>(
std::vector<std::string>{"user@example.com"}); std::vector<std::string>{"user@example.com"});
auto store{Store::make_new(tempdir.path(), MuTestMaildir, conf)}; auto store{Store::make_new(tempdir.path(), MuTestMaildir, conf)};

View File

@ -188,18 +188,22 @@ namespace Mu::Scm {
if (!pred) if (!pred)
throw ScmError{ScmError::Id::WrongType, func, pos, ARG, expected}; throw ScmError{ScmError::Id::WrongType, func, pos, ARG, expected};
}; };
// note: use the C predicates (scm_is_string etc.); the Scheme
// predicates (scm_string_p etc.) return an SCM boolean, which
// is truthy as a C++ bool even when it is #f.
using Type = std::remove_const_t<T>; // *not* std::remove_const using Type = std::remove_const_t<T>; // *not* std::remove_const
if constexpr (std::is_same_v<Type, std::string>) { if constexpr (std::is_same_v<Type, std::string>) {
ensure(scm_string_p(ARG), ARG, "string"); ensure(scm_is_string(ARG), ARG, "string");
auto str{scm_to_utf8_string(ARG)}; size_t len{};
std::string res{str}; auto str{scm_to_utf8_stringn(ARG, &len)};
std::string res{str, len};
::free(str); ::free(str);
return res; return res;
} else if constexpr (std::is_same_v<Type, char>) { } else if constexpr (std::is_same_v<Type, char>) {
ensure(scm_char_p(ARG), ARG, "character"); ensure(SCM_CHARP(ARG), ARG, "character");
return scm_to_char(ARG); return static_cast<char>(SCM_CHAR(ARG));
} else if constexpr (std::is_same_v<Type, bool>) { } else if constexpr (std::is_same_v<Type, bool>) {
ensure(scm_boolean_p(ARG), ARG, "bool"); ensure(scm_is_bool(ARG), ARG, "bool");
return scm_to_bool(ARG); return scm_to_bool(ARG);
} else if constexpr (std::is_same_v<Type, int>) { } else if constexpr (std::is_same_v<Type, int>) {
ensure(scm_is_signed_integer(ARG, std::numeric_limits<int>::min(), ensure(scm_is_signed_integer(ARG, std::numeric_limits<int>::min(),
@ -292,7 +296,7 @@ namespace Mu::Scm {
*/ */
template<typename Key, typename Value, typename... KeyVals> template<typename Key, typename Value, typename... KeyVals>
static inline SCM alist_add(SCM alist, const Key& key, const Value& val, static inline SCM alist_add(SCM alist, const Key& key, const Value& val,
KeyVals... keyvals) { KeyVals&&... keyvals) {
SCM res = scm_acons(to_scm(key), to_scm(val), alist); SCM res = scm_acons(to_scm(key), to_scm(val), alist);
return alist_add(res, std::forward<KeyVals>(keyvals)...); return alist_add(res, std::forward<KeyVals>(keyvals)...);
} }