From 62d56aafff9a24fa3d52ce6dd201435b7db28b51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Mangano-Tarumi?= Date: Sun, 18 Nov 2018 11:04:11 -0500 Subject: [PATCH] accompany returned status codes with a message --- src/cli.cc | 5 +++-- src/ogg.cc | 34 ++++++++++++++++++---------------- src/opus.cc | 25 +++++++++++++++---------- src/opustags.cc | 2 +- src/opustags.h | 22 +++++++++++----------- t/opus.cc | 16 ++++++++-------- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/cli.cc b/src/cli.cc index e7d7ae8..8e95da0 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -211,8 +211,9 @@ std::list ot::read_comments(FILE* input) static ot::status process_tags(const ogg_packet& packet, const ot::options& opt, ot::ogg_writer* writer) { ot::opus_tags tags; - if(ot::parse_tags(packet, tags) != ot::st::ok) - return ot::st::bad_comment_header; + ot::status rc = ot::parse_tags(packet, tags); + if (rc != ot::st::ok) + return rc; if (opt.delete_all) { tags.comments.clear(); diff --git a/src/ogg.cc b/src/ogg.cc index 760a5db..c8220c3 100644 --- a/src/ogg.cc +++ b/src/ogg.cc @@ -7,6 +7,8 @@ #include +using namespace std::literals::string_literals; + ot::ogg_reader::ogg_reader(FILE* input) : file(input) { @@ -24,22 +26,22 @@ ot::status ot::ogg_reader::read_page() { while (ogg_sync_pageout(&sync, &page) != 1) { if (feof(file)) - return st::end_of_stream; + return {st::end_of_stream, "End of stream was reached"}; char* buf = ogg_sync_buffer(&sync, 65536); if (buf == nullptr) - return st::libogg_error; + return {st::libogg_error, "ogg_sync_buffer failed"}; size_t len = fread(buf, 1, 65536, file); if (ferror(file)) - return st::standard_error; + return {st::standard_error, "fread error: "s + strerror(errno)}; if (ogg_sync_wrote(&sync, len) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_sync_wrote failed"}; if (ogg_sync_check(&sync) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_sync_check failed"}; } /* at this point, we've got a good page */ if (!stream_ready) { if (ogg_stream_init(&stream, ogg_page_serialno(&page)) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_stream_init failed"}; stream_ready = true; } stream_in_sync = false; @@ -49,19 +51,19 @@ ot::status ot::ogg_reader::read_page() ot::status ot::ogg_reader::read_packet() { if (!stream_ready) - return st::stream_not_ready; + return {st::stream_not_ready, "Stream was not initialized"}; if (!stream_in_sync) { if (ogg_stream_pagein(&stream, &page) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_stream_pagein failed"}; stream_in_sync = true; } int rc = ogg_stream_packetout(&stream, &packet); if (rc == 1) return st::ok; else if (rc == 0 && ogg_stream_check(&stream) == 0) - return st::end_of_page; + return {st::end_of_page, "End of page was reached"}; else - return st::libogg_error; + return {st::libogg_error, "ogg_stream_packetout failed"}; } ot::ogg_writer::ogg_writer(FILE* output) @@ -79,13 +81,13 @@ ot::ogg_writer::~ogg_writer() ot::status ot::ogg_writer::write_page(const ogg_page& page) { if (page.header_len < 0 || page.body_len < 0) - return st::int_overflow; + return {st::int_overflow, "Overflowing page length"}; auto header_len = static_cast(page.header_len); auto body_len = static_cast(page.body_len); if (fwrite(page.header, 1, header_len, file) < header_len) - return st::standard_error; + return {st::standard_error, "fwrite error: "s + strerror(errno)}; if (fwrite(page.body, 1, body_len, file) < body_len) - return st::standard_error; + return {st::standard_error, "fwrite error: "s + strerror(errno)}; return st::ok; } @@ -93,7 +95,7 @@ ot::status ot::ogg_writer::prepare_stream(long serialno) { if (stream.serialno != serialno) { if (ogg_stream_reset_serialno(&stream, serialno) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_stream_reset_serialno failed"}; } return st::ok; } @@ -101,7 +103,7 @@ ot::status ot::ogg_writer::prepare_stream(long serialno) ot::status ot::ogg_writer::write_packet(const ogg_packet& packet) { if (ogg_stream_packetin(&stream, const_cast(&packet)) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_stream_packetin failed"}; else return st::ok; } @@ -112,6 +114,6 @@ ot::status ot::ogg_writer::flush_page() if (ogg_stream_flush(&stream, &page) != 0) return write_page(page); if (ogg_stream_check(&stream) != 0) - return st::libogg_error; + return {st::libogg_error, "ogg_stream_check failed"}; return st::ok; /* nothing was done */ } diff --git a/src/opus.cc b/src/opus.cc index 5b211eb..1531624 100644 --- a/src/opus.cc +++ b/src/opus.cc @@ -40,9 +40,11 @@ ot::status ot::validate_identification_header(const ogg_packet& packet) { if (packet.bytes < 8) - return ot::st::bad_identification_header; + return {ot::st::cut_magic_number, + "Identification header too short for the magic number"}; if (memcmp(packet.packet, "OpusHead", 8) != 0) - return ot::st::bad_identification_header; + return {ot::st::bad_magic_number, + "Identification header did not start with OpusHead"}; return ot::st::ok; } @@ -52,7 +54,7 @@ ot::status ot::validate_identification_header(const ogg_packet& packet) ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags) { if (packet.bytes < 0) - return st::int_overflow; + return {st::int_overflow, "Overflowing comment header length"}; size_t size = static_cast(packet.bytes); const char* data = reinterpret_cast(packet.packet); size_t pos = 0; @@ -60,33 +62,36 @@ ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags) // Magic number if (8 > size) - return st::overflowing_magic_number; + return {st::cut_magic_number, "Comment header too short for the magic number"}; if (memcmp(data, "OpusTags", 8) != 0) - return st::bad_magic_number; + return {st::bad_magic_number, "Comment header did not start with OpusTags"}; // Vendor pos = 8; if (pos + 4 > size) - return st::overflowing_vendor_length; + return {st::cut_vendor_length, + "Vendor string length did not fit the comment header"}; size_t vendor_length = le32toh(*((uint32_t*) (data + pos))); if (pos + 4 + vendor_length > size) - return st::overflowing_vendor_data; + return {st::cut_vendor_data, "Vendor string did not fit the comment header"}; my_tags.vendor = std::string(data + pos + 4, vendor_length); pos += 4 + my_tags.vendor.size(); // Comment count if (pos + 4 > size) - return st::overflowing_comment_count; + return {st::cut_comment_count, "Comment count did not fit the comment header"}; uint32_t count = le32toh(*((uint32_t*) (data + pos))); pos += 4; // Comments' data for (uint32_t i = 0; i < count; ++i) { if (pos + 4 > size) - return st::overflowing_comment_length; + return {st::cut_comment_length, + "Comment length did not fit the comment header"}; uint32_t comment_length = le32toh(*((uint32_t*) (data + pos))); if (pos + 4 + comment_length > size) - return st::overflowing_comment_data; + return {st::cut_comment_data, + "Comment string did not fit the comment header"}; const char *comment_value = data + pos + 4; my_tags.comments.emplace_back(comment_value, comment_length); pos += 4 + comment_length; diff --git a/src/opustags.cc b/src/opustags.cc index 578e4a2..73ebc7c 100644 --- a/src/opustags.cc +++ b/src/opustags.cc @@ -14,5 +14,5 @@ int main(int argc, char** argv) { else if (rc != ot::st::ok) return EXIT_FAILURE; rc = run(opt); - return rc == ot::st::ok ? EXIT_SUCCESS : EXIT_FAILURE; + return (rc == ot::st::ok || rc == ot::st::exit_now) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/src/opustags.h b/src/opustags.h index 11f3756..d9e0f36 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -18,9 +18,9 @@ namespace ot { /** * Possible return status. * - * The overflowing error family means that the end of packet was reached when attempting to read the - * overflowing value. For example, overflowing_comment_count means that after reading the vendor - * string, less than 4 bytes were left in the packet. + * The cut error family means that the end of packet was reached when attempting to read the + * overflowing value. For example, cut_comment_count means that after reading the vendor string, + * less than 4 bytes were left in the packet. */ enum class st { /* Generic */ @@ -33,15 +33,13 @@ enum class st { stream_not_ready, libogg_error, /* Opus */ - bad_identification_header, - bad_comment_header, bad_magic_number, - overflowing_magic_number, - overflowing_vendor_length, - overflowing_vendor_data, - overflowing_comment_count, - overflowing_comment_length, - overflowing_comment_data, + cut_magic_number, + cut_vendor_length, + cut_vendor_data, + cut_comment_count, + cut_comment_length, + cut_comment_data, /* CLI */ bad_arguments, exit_now, /**< The program should terminate successfully. */ @@ -51,6 +49,8 @@ enum class st { /** * Wraps a status code with an optional message. It is implictly converted from and to a * #status_code. + * + * All the error statuses should be accompanied with a relevant error message. */ struct status { status(st code = st::ok) : code(code) {} diff --git a/t/opus.cc b/t/opus.cc index b5e6ab4..a575ee5 100644 --- a/t/opus.cc +++ b/t/opus.cc @@ -22,12 +22,12 @@ static void check_identification() throw failure("did not accept a good OpusHead"); packet.bytes = 7; - if (ot::validate_identification_header(packet) != ot::st::bad_identification_header) + if (ot::validate_identification_header(packet) != ot::st::cut_magic_number) throw failure("accepted an OpusHead that is too short"); packet.packet = (unsigned char*) "NotOpusHead"; packet.bytes = 11; - if (ot::validate_identification_header(packet) != ot::st::bad_identification_header) + if (ot::validate_identification_header(packet) != ot::st::bad_magic_number) throw failure("did not report the right status for a bad OpusHead"); } @@ -85,10 +85,10 @@ static void parse_corrupted() char* end = packet + size; op.bytes = 7; - if (ot::parse_tags(op, tags) != ot::st::overflowing_magic_number) + if (ot::parse_tags(op, tags) != ot::st::cut_magic_number) throw failure("did not detect the overflowing magic number"); op.bytes = 11; - if (ot::parse_tags(op, tags) != ot::st::overflowing_vendor_length) + if (ot::parse_tags(op, tags) != ot::st::cut_vendor_length) throw failure("did not detect the overflowing vendor string length"); op.bytes = size; @@ -98,18 +98,18 @@ static void parse_corrupted() header_data[0] = 'O'; *vendor_length = end - vendor_string + 1; - if (ot::parse_tags(op, tags) != ot::st::overflowing_vendor_data) + if (ot::parse_tags(op, tags) != ot::st::cut_vendor_data) throw failure("did not detect the overflowing vendor string"); *vendor_length = end - vendor_string - 3; - if (ot::parse_tags(op, tags) != ot::st::overflowing_comment_count) + if (ot::parse_tags(op, tags) != ot::st::cut_comment_count) throw failure("did not detect the overflowing comment count"); *vendor_length = comment_count - vendor_string; ++*comment_count; - if (ot::parse_tags(op, tags) != ot::st::overflowing_comment_length) + if (ot::parse_tags(op, tags) != ot::st::cut_comment_length) throw failure("did not detect the overflowing comment length"); *first_comment_length = end - first_comment_data + 1; - if (ot::parse_tags(op, tags) != ot::st::overflowing_comment_data) + if (ot::parse_tags(op, tags) != ot::st::cut_comment_data) throw failure("did not detect the overflowing comment data"); }