From d453af2563f19451aa31096de58db73214ae75d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Mangano?= Date: Sun, 17 Jan 2021 15:43:16 +0100 Subject: [PATCH] Migrate the system module to use exceptions --- src/cli.cc | 66 ++++++++++++++++++++++---------------------------- src/opustags.h | 10 ++++---- src/system.cc | 60 +++++++++++++++++++++------------------------ t/system.cc | 32 ++++++++++++------------ 4 files changed, 78 insertions(+), 90 deletions(-) diff --git a/src/cli.cc b/src/cli.cc index 10269c8..012f86b 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -62,7 +62,6 @@ ot::options ot::parse_options(int argc, char** argv, FILE* comments_input) { options opt; static ot::encoding_converter to_utf8("", "UTF-8"); - std::string utf8; const char* equal; ot::status rc; bool set_all = false; @@ -133,11 +132,11 @@ ot::options ot::parse_options(int argc, char** argv, FILE* comments_input) // Convert arguments to UTF-8. if (!opt.raw) { for (std::list* args : { &opt.to_add, &opt.to_delete }) { - for (std::string& arg : *args) { - rc = to_utf8(arg, utf8); - if (rc != ot::st::ok) - throw status {st::bad_arguments, "Could not encode argument into UTF-8: " + rc.message}; - arg = std::move(utf8); + try { + for (std::string& arg : *args) + arg = to_utf8(arg); + } catch (const ot::status& rc) { + throw status {st::bad_arguments, "Could not encode argument into UTF-8: " + rc.message}; } } } @@ -190,11 +189,12 @@ void ot::print_comments(const std::list& comments, FILE* output, bo if (raw) { comment = &utf8_comment; } else { - ot::status rc = from_utf8(utf8_comment, local); - comment = &local; - if (rc != ot::st::ok) { + try { + local = from_utf8(utf8_comment); + comment = &local; + } catch (ot::status& rc) { rc.message += " See --raw."; - throw rc; + throw; } } @@ -236,11 +236,9 @@ std::list ot::read_comments(FILE* input, bool raw) if (raw) { comments.emplace_back(line, nread); } else { - std::string utf8; - ot::status rc = to_utf8(std::string_view(line, nread), utf8); - if (rc == ot::st::ok) { - comments.emplace_back(std::move(utf8)); - } else { + try { + comments.emplace_back(to_utf8(std::string_view(line, nread))); + } catch (const ot::status& rc) { free(line); throw ot::status {ot::st::badly_encoded, "UTF-8 conversion error: " + rc.message}; } @@ -310,12 +308,15 @@ static void edit_tags_interactively(ot::opus_tags& tags, const std::optionalc_str(), "we")) == nullptr) - rc = {ot::st::standard_error, - "Could not open '" + path_out.value() + "' for writing: " + - strerror(errno)}; + throw ot::status {ot::st::standard_error, + "Could not open '" + path_out.value() + "' for writing: " + strerror(errno)}; output = final_output.get(); } else if (opt.overwrite) { - rc = temporary_output.open(path_out->c_str()); + temporary_output.open(path_out->c_str()); output = temporary_output.get(); } else { - rc = {ot::st::error, - "'" + path_out.value() + "' already exists. Use -y to overwrite."}; + throw ot::status {ot::st::error, "'" + path_out.value() + "' already exists. Use -y to overwrite."}; } } else if (errno == ENOENT) { - rc = temporary_output.open(path_out->c_str()); + temporary_output.open(path_out->c_str()); output = temporary_output.get(); } else { - rc = {ot::st::error, - "Could not identify '" + path_in + "': " + strerror(errno)}; + throw ot::status {ot::st::error, "Could not identify '" + path_in + "': " + strerror(errno)}; } - if (rc != ot::st::ok) - throw rc; ot::ogg_writer writer(output); writer.path = path_out; process(reader, &writer, opt); - - rc = temporary_output.commit(); - if (rc != ot::st::ok) - throw rc; + temporary_output.commit(); } void ot::run(const ot::options& opt) diff --git a/src/opustags.h b/src/opustags.h index 19551cc..43a54c8 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -128,9 +128,9 @@ public: * temporary file is created in the same directory as its destination in order to make the * final move operation instant. */ - ot::status open(const char* destination); + void open(const char* destination); /** Close then move the partial file to its final location. */ - ot::status commit(); + void commit(); /** Delete the temporary file. */ void abort(); /** Get the underlying FILE* handle. */ @@ -158,7 +158,7 @@ public: * Convert text using iconv. If the input sequence is invalid, return #st::badly_encoded and * abort the processing, leaving out in an undefined state. */ - status operator()(std::string_view in, std::string& out); + std::string operator()(std::string_view in); private: iconv_t cd; /**< conversion descriptor */ }; @@ -173,13 +173,13 @@ std::string shell_escape(std::string_view word); * editor is passed unescaped to the shell, and may contain CLI options. * path is the name of the file to edit, which will be passed as the last argument to editor. */ -ot::status run_editor(std::string_view editor, std::string_view path); +void run_editor(std::string_view editor, std::string_view path); /** * Return the specified path’s mtime, i.e. the last data modification * timestamp. */ -ot::status get_file_timestamp(const char* path, timespec& mtime); +timespec get_file_timestamp(const char* path); /** \} */ diff --git a/src/system.cc b/src/system.cc index 4e37859..18bb395 100644 --- a/src/system.cc +++ b/src/system.cc @@ -20,22 +20,20 @@ using namespace std::string_literals; -ot::status ot::partial_file::open(const char* destination) +void ot::partial_file::open(const char* destination) { - abort(); final_name = destination; temporary_name = final_name + ".XXXXXX.part"; int fd = mkstemps(const_cast(temporary_name.data()), 5); if (fd == -1) - return {st::standard_error, - "Could not create a partial file for '" + final_name + "': " + - strerror(errno)}; + throw status {st::standard_error, + "Could not create a partial file for '" + final_name + "': " + + strerror(errno)}; file = fdopen(fd, "w"); if (file == nullptr) - return {st::standard_error, - "Could not get the partial file handle to '" + temporary_name + "': " + - strerror(errno)}; - return st::ok; + throw status {st::standard_error, + "Could not get the partial file handle to '" + temporary_name + "': " + + strerror(errno)}; } static mode_t get_umask() @@ -71,17 +69,16 @@ static void copy_permissions(const char* source, const char* dest) fprintf(stderr, "warning: Could not set mode of %s: %s\n", dest, strerror(errno)); } -ot::status ot::partial_file::commit() +void ot::partial_file::commit() { if (file == nullptr) - return st::ok; + return; file.reset(); copy_permissions(final_name.c_str(), temporary_name.c_str()); if (rename(temporary_name.c_str(), final_name.c_str()) == -1) - return {st::standard_error, - "Could not move the result file '" + temporary_name + "' to '" + - final_name + "': " + strerror(errno) + "."}; - return st::ok; + throw status {st::standard_error, + "Could not move the result file '" + temporary_name + "' to '" + + final_name + "': " + strerror(errno) + "."}; } void ot::partial_file::abort() @@ -104,10 +101,10 @@ ot::encoding_converter::~encoding_converter() iconv_close(cd); } -ot::status ot::encoding_converter::operator()(std::string_view in, std::string& out) +std::string ot::encoding_converter::operator()(std::string_view in) { iconv(cd, nullptr, nullptr, nullptr, nullptr); - out.clear(); + std::string out; out.reserve(in.size()); char* in_cursor = const_cast(in.data()); size_t in_left = in.size(); @@ -121,10 +118,10 @@ ot::status ot::encoding_converter::operator()(std::string_view in, std::string& if (rc == (size_t) -1 && errno == E2BIG) { // Loop normally. } else if (rc == (size_t) -1) { - return {ot::st::badly_encoded, strerror(errno) + "."s}; + throw status {ot::st::badly_encoded, strerror(errno) + "."s}; } else if (rc != 0) { - return {ot::st::badly_encoded, - "Some characters could not be converted into the target encoding."}; + throw status {ot::st::badly_encoded, + "Some characters could not be converted into the target encoding."}; } out.append(chunk, out_cursor - chunk); @@ -133,7 +130,7 @@ ot::status ot::encoding_converter::operator()(std::string_view in, std::string& else if (in_left == 0) in_cursor = nullptr; } - return ot::st::ok; + return out; } std::string ot::shell_escape(std::string_view word) @@ -156,28 +153,27 @@ std::string ot::shell_escape(std::string_view word) return escaped_word; } -ot::status ot::run_editor(std::string_view editor, std::string_view path) +void ot::run_editor(std::string_view editor, std::string_view path) { std::string command = std::string(editor) + " " + shell_escape(path); int status = system(command.c_str()); if (status == -1) - return {st::standard_error, "waitpid error: "s + strerror(errno)}; + throw ot::status {st::standard_error, "waitpid error: "s + strerror(errno)}; else if (!WIFEXITED(status)) - return {st::child_process_failed, - "Child process did not terminate normally: "s + strerror(errno)}; + throw ot::status {st::child_process_failed, + "Child process did not terminate normally: "s + strerror(errno)}; else if (WEXITSTATUS(status) != 0) - return {st::child_process_failed, - "Child process exited with " + std::to_string(WEXITSTATUS(status))}; - - return st::ok; + throw ot::status {st::child_process_failed, + "Child process exited with " + std::to_string(WEXITSTATUS(status))}; } -ot::status ot::get_file_timestamp(const char* path, timespec& mtime) +timespec ot::get_file_timestamp(const char* path) { + timespec mtime; struct stat st; if (stat(path, &st) == -1) - return {st::standard_error, path + ": stat error: "s + strerror(errno)}; + throw status {st::standard_error, path + ": stat error: "s + strerror(errno)}; #if defined(HAVE_STAT_ST_MTIM) mtime = st.st_mtim; #elif defined(HAVE_STAT_ST_MTIMESPEC) @@ -186,5 +182,5 @@ ot::status ot::get_file_timestamp(const char* path, timespec& mtime) mtime.tv_sec = st.st_mtime; mtime.tv_nsec = st.st_mtimensec; #endif - return st::ok; + return mtime; } diff --git a/t/system.cc b/t/system.cc index 49c5a64..8563d1d 100644 --- a/t/system.cc +++ b/t/system.cc @@ -10,10 +10,14 @@ void check_partial_files() std::string name; { ot::partial_file bad_tmp; - is(bad_tmp.open("/dev/null"), ot::st::standard_error, - "opening a device as a partial file fails"); - is(bad_tmp.open(result), ot::st::ok, - "opening a regular partial file works"); + try { + bad_tmp.open("/dev/null"); + throw failure("opening a device as a partial file should fail"); + } catch (const ot::status& rc) { + is(rc, ot::st::standard_error, "opening a device as a partial file fails"); + } + + bad_tmp.open(result); name = bad_tmp.name(); if (name.size() != strlen(result) + 12 || name.compare(0, strlen(result), result) != 0) @@ -22,9 +26,9 @@ void check_partial_files() is(access(name.c_str(), F_OK), -1, "expect the temporary file is deleted"); ot::partial_file good_tmp; - is(good_tmp.open(result), ot::st::ok, "open the partial file"); + good_tmp.open(result); name = good_tmp.name(); - is(good_tmp.commit(), ot::st::ok, "commit the result file"); + good_tmp.commit(); is(access(name.c_str(), F_OK), -1, "expect the temporary file is deleted"); is(access(result, F_OK), 0, "expect the final result file"); is(remove(result), 0, "remove the result file"); @@ -35,18 +39,14 @@ void check_converter() const char* ephemere_iso = "\xc9\x70\x68\xe9\x6d\xe8\x72\x65"; ot::encoding_converter to_utf8("ISO_8859-1", "UTF-8"); ot::encoding_converter from_utf8("UTF-8", "ISO_8859-1//IGNORE"); - std::string out; - ot::status rc = to_utf8(ephemere_iso, out); - is(rc, ot::st::ok, "conversion to UTF-8 is successful"); - is(out, "Éphémère", "conversion to UTF-8 is correct"); + is(to_utf8(ephemere_iso), "Éphémère", "conversion to UTF-8 is correct"); + is(from_utf8("Éphémère"), ephemere_iso, "conversion from UTF-8 is correct"); - rc = from_utf8("Éphémère", out); - is(rc, ot::st::ok, "conversion from UTF-8 is successful"); - is(out, ephemere_iso, "conversion from UTF-8 is correct"); - - rc = from_utf8("\xFF\xFF", out); - is(rc, ot::st::badly_encoded, "conversion from bad UTF-8 fails"); + try { + from_utf8("\xFF\xFF"); + throw failure("conversion from bad UTF-8 did not fail"); + } catch (const ot::status&) {} } void check_shell_esape()