From 1e1311fc3970c278db4bd7852f3fa317c8485c81 Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Thu, 23 Jul 2026 17:19:28 +0300 Subject: [PATCH] mu-utils: cleanups Fix some types Improve error handling Remove dead/unused code --- lib/utils/mu-utils-file.cc | 28 ++++++++----- lib/utils/mu-utils.cc | 16 +++++--- lib/utils/mu-utils.hh | 75 ----------------------------------- lib/utils/tests/test-utils.cc | 2 + 4 files changed, 32 insertions(+), 89 deletions(-) diff --git a/lib/utils/mu-utils-file.cc b/lib/utils/mu-utils-file.cc index 735fe2c3..72216b25 100644 --- a/lib/utils/mu-utils-file.cc +++ b/lib/utils/mu-utils-file.cc @@ -130,8 +130,8 @@ Mu::remove_directory(const std::string& path) std::error_code err{}; const auto n{std::filesystem::remove_all(path, err)}; if (err) - return Err(Error::Code::File, "failed to remove {}; exit-code={}", - path, err.value()); + return Err(Error::Code::File, "failed to remove {}: {}", + path, err.message()); mu_debug("removed directory '{}' ({})", path, n); @@ -228,7 +228,7 @@ Mu::run_command(std::initializer_list args, bool try_setsid) int wait_status{}; gchar *std_out{}, *std_err{}; auto res = g_spawn_sync({}, - static_cast(argvec.data()), + argvec.data(), {}, (GSpawnFlags)(G_SPAWN_SEARCH_PATH), try_setsid ? maybe_setsid : nullptr, {}, @@ -239,11 +239,21 @@ Mu::run_command(std::initializer_list args, bool try_setsid) if (!res) return Err(Error::Code::File, &err, "failed to execute command"); - else - return Ok(Mu::CommandOutput{ - WEXITSTATUS(wait_status), - to_string_gchar(std::move(std_out/*consumed*/)), - to_string_gchar(std::move(std_err/*consumed*/))}); + + /* a process killed by a signal has no meaningful exit-code; flag it + * as an error rather than reporting a bogus WEXITSTATUS. */ + if (!WIFEXITED(wait_status)) { + auto oops{Err(Error::Code::File, "command terminated abnormally " + "(wait-status={})", wait_status)}; + g_free(std_out); + g_free(std_err); + return oops; + } + + return Ok(Mu::CommandOutput{ + WEXITSTATUS(wait_status), + to_string_gchar(std::move(std_out/*consumed*/)), + to_string_gchar(std::move(std_err/*consumed*/))}); } Result @@ -304,7 +314,7 @@ Mu::play (const std::string& path) } /* LCOV_EXCL_STOP*/ -Result +static Result expand_path_real(const std::string& str) { #ifndef HAVE_WORDEXP_H diff --git a/lib/utils/mu-utils.cc b/lib/utils/mu-utils.cc index f0faae40..145adca7 100644 --- a/lib/utils/mu-utils.cc +++ b/lib/utils/mu-utils.cc @@ -168,7 +168,7 @@ asciify_in_place (char *buf) g_return_val_if_fail (buf, NULL); for (c = buf; c && *c; ++c) { - if ((!isprint(*c) && !isspace (*c)) || !isascii(*c)) + if (!is_ascii(*c) || (is_ascii_cntrl(*c) && !is_ascii_space(*c))) *c = '.'; } @@ -321,7 +321,7 @@ Mu::split(const std::string& str, char sepa) while (true) { if (e = str.find(sepa, b); e != std::string::npos) { vec.emplace_back(str.substr(b, e - b)); - b = e + sizeof(sepa); + b = e + 1; } else { vec.emplace_back(str.substr(b)); break; @@ -340,7 +340,7 @@ Mu::join(const std::vector& svec, const std::string& sepa) /* calculate the overall size beforehand, to avoid re-allocations. */ size_t value_len = - std::accumulate(svec.cbegin(), svec.cend(), 0, + std::accumulate(svec.cbegin(), svec.cend(), std::size_t{}, [](size_t size, const std::string& s) { return size + s.size(); }) + (svec.size() - 1) * sepa.length(); @@ -504,7 +504,8 @@ Mu::parse_date_time(const std::string& dstr, bool is_first, bool utc) constexpr char UserDateMax[] = "29991231235959"; std::string date(is_first ? UserDateMin : UserDateMax); - std::copy_if(dstr.begin(), dstr.end(), date.begin(), [](auto c) { return isdigit(c); }); + std::copy_if(dstr.begin(), dstr.end(), date.begin(), + [](auto c) { return is_ascii_digit(c); }); if (!::strptime(date.c_str(), "%Y%m%d%H%M%S", &tbuf) && !::strptime(date.c_str(), "%Y%m%d%H%M", &tbuf) && @@ -550,7 +551,12 @@ Mu::parse_size(const std::string& val, bool is_first) if (!groups) return Nothing; - int64_t size{::atoll(groups->at(1).c_str())}; // check overflow? + int64_t size{}; + const auto& digits{groups->at(1)}; + if (const auto res = std::from_chars(digits.data(), + digits.data() + digits.size(), size); + res.ec != std::errc{}) + return Nothing; // overflow or not-a-number const auto& unit{groups->at(2)}; switch (unit.empty() ? 0 : g_ascii_tolower(unit.at(0))) { diff --git a/lib/utils/mu-utils.hh b/lib/utils/mu-utils.hh index e84ca272..93efc105 100644 --- a/lib/utils/mu-utils.hh +++ b/lib/utils/mu-utils.hh @@ -78,11 +78,6 @@ void mu_log(GLogLevelFlags level, fmt::format_string frm, T&&... args) noe g_log("mu", level, "%s", mu_format(frm, std::forward(args)...).c_str()); } -template -void mu_none(fmt::format_string, T&&...) noexcept { - // ignore -} - template void mu_debug(fmt::format_string frm, T&&... args) noexcept { mu_log(G_LOG_LEVEL_DEBUG, frm, std::forward(args)...); @@ -132,17 +127,6 @@ void mu_printerrln(fmt::format_string frm, T&&... args) noexcept { fmt::println(stderr, frm, std::forward(args)...); } -// null-stream -class NullStream : public std::ostream { -public: - NullStream() : std::ostream(&buf_) {} -private: - struct NullBuffer : public std::streambuf { - int overflow(int c) override { return c; } - }; - NullBuffer buf_; -}; - /* stream print */ template void mu_print(std::ostream& os, fmt::format_string frm, T&&... args) noexcept { @@ -518,11 +502,6 @@ to_unit(Duration d) return duration_cast(d).count(); } -constexpr int64_t -to_s(Duration d) -{ - return to_unit(d); -} constexpr int64_t to_ms(Duration d) { @@ -563,16 +542,6 @@ private: */ Option parse_size(const std::string& sizestr, bool first); -/** - * Convert a size into a size in bytes string - * - * @param size the size - * @param first - * - * @return the size expressed as a string with the decimal number of bytes - */ -std::string size_to_string(int64_t size); - /** * get a crude 'summary' of the string, ie. the first /n/ lines of the strings, * with all newlines removed, replaced by single spaces @@ -649,21 +618,6 @@ to_string_gchar(gchar*&& str) g_free(str); return s; } -/** - * Consume a char* and return a std::string - * - * @param str a gchar* (consumed/freed with ::free()) - * - * @return a std::string, empty if gchar was {} - */ -inline std::string -to_string_char(char*&& str) -{ - std::string s(str?str:""); - ::free(str); - return s; -} - /** * Shell-quote the given string (as per g_shell_quote()) * @@ -728,21 +682,6 @@ bool seq_some(const Sequence& seq, UnaryPredicate pred) { return seq_find_if(seq, pred) != seq.cend(); } -/** - * Create a sequence that has all element of seq for which pred is true - * - * @param seq sequence - * @param pred false - * - * @return sequence - */ -template -Sequence seq_filter(const Sequence& seq, UnaryPredicate pred) { - Sequence res; - std::copy_if(seq.begin(), seq.end(), std::back_inserter(res), pred); - return res; -} - /** * Create a sequence that has all element of seq for which pred is false * @@ -762,20 +701,6 @@ template void seq_sort(Sequence& seq, Compare cmp) { std::sort(seq.begin(), seq.end(), cmp); } -/** - * Like std::accumulate, but using a sequence instead of a range. - * - * @param seq some std::accumulate compatible sequence - * @param init the initial value - * @param op binary operation to calculate the next element - * - * @return the result value. - */ -template -ResultType seq_fold(const Sequence& seq, ResultType init, BinaryOp op) { - return std::accumulate(seq.cbegin(), seq.cend(), init, op); -} - template void seq_for_each(const Sequence& seq, UnaryOp op) { std::for_each(seq.cbegin(), seq.cend(), op); diff --git a/lib/utils/tests/test-utils.cc b/lib/utils/tests/test-utils.cc index 1ff4b9ff..34238258 100644 --- a/lib/utils/tests/test-utils.cc +++ b/lib/utils/tests/test-utils.cc @@ -148,6 +148,8 @@ test_parse_size() g_assert_false(!!parse_size("-1", true)); g_assert_false(!!parse_size("scoobydoobydoo", false)); + /* overflows int64 -> Nothing rather than a wrapped value */ + g_assert_false(!!parse_size("99999999999999999999", false)); } static void