mu-utils: cleanups

Fix some types

Improve error handling

Remove dead/unused code

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<std::string> args, bool try_setsid)
 	int wait_status{};
 	gchar *std_out{}, *std_err{};
 	auto res = g_spawn_sync({},
-				static_cast<char**>(argvec.data()),
+				argvec.data(),
 				{},
 				(GSpawnFlags)(G_SPAWN_SEARCH_PATH),
 				try_setsid ? maybe_setsid : nullptr, {},
@@ -239,11 +239,21 @@ Mu::run_command(std::initializer_list<std::string> 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<Mu::CommandOutput>
@@ -304,7 +314,7 @@ Mu::play (const std::string& path)
 }
 /* LCOV_EXCL_STOP*/
 
-Result<std::string>
+static Result<std::string>
 expand_path_real(const std::string& str)
 {
 #ifndef HAVE_WORDEXP_H

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<std::string>& 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))) {

lib/utils/mu-utils.hh

@@ -78,11 +78,6 @@ void mu_log(GLogLevelFlags level, fmt::format_string<T...> frm, T&&... args) noe
 	g_log("mu", level, "%s", mu_format(frm, std::forward<T>(args)...).c_str());
 }
 
-template<typename...T>
-void mu_none(fmt::format_string<T...>, T&&...) noexcept {
-	// ignore
-}
-
 template<typename...T>
 void mu_debug(fmt::format_string<T...> frm, T&&... args) noexcept {
 	mu_log(G_LOG_LEVEL_DEBUG, frm, std::forward<T>(args)...);
@@ -132,17 +127,6 @@ void mu_printerrln(fmt::format_string<T...> frm, T&&... args) noexcept {
 	fmt::println(stderr, frm, std::forward<T>(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<typename...T>
 void mu_print(std::ostream& os, fmt::format_string<T...> frm, T&&... args) noexcept {
@@ -518,11 +502,6 @@ to_unit(Duration d)
 	return duration_cast<Unit>(d).count();
 }
 
-constexpr int64_t
-to_s(Duration d)
-{
-	return to_unit<std::chrono::seconds>(d);
-}
 constexpr int64_t
 to_ms(Duration d)
 {
@@ -563,16 +542,6 @@ private:
  */
 Option<int64_t> 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<typename Sequence, typename UnaryPredicate>
-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<typename Sequence, typename Compare>
 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<typename Sequence, typename ResultType,  typename BinaryOp>
-ResultType seq_fold(const Sequence& seq, ResultType init, BinaryOp op) {
-	return std::accumulate(seq.cbegin(), seq.cend(), init, op);
-}
-
 template<typename Sequence, typename UnaryOp>
 void seq_for_each(const Sequence& seq, UnaryOp op) {
 	std::for_each(seq.cbegin(), seq.cend(), op);

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