diff --git a/src/cli.cc b/src/cli.cc index 05de3d8..10269c8 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -374,13 +374,7 @@ static void process(ot::ogg_reader& reader, ot::ogg_writer* writer, const ot::op writer->write_page(reader.page); } else if (reader.absolute_page_no == 1) { // Comment header ot::opus_tags tags; - reader.process_header_packet( - [&tags](ogg_packet& p) { - ot::status rc = ot::parse_tags(p, tags); - if (rc != ot::st::ok) - throw rc; - } - ); + reader.process_header_packet([&tags](ogg_packet& p) { tags = ot::parse_tags(p); }); edit_tags(tags, opt); if (writer) { if (opt.edit_interactively) { diff --git a/src/opus.cc b/src/opus.cc index ed8e49b..1b38a2e 100644 --- a/src/opus.cc +++ b/src/opus.cc @@ -39,10 +39,10 @@ #define le32toh(x) OSSwapLittleToHostInt32(x) #endif -ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags) +ot::opus_tags ot::parse_tags(const ogg_packet& packet) { if (packet.bytes < 0) - return {st::int_overflow, "Overflowing comment header length"}; + throw status {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; @@ -50,36 +50,36 @@ ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags) // Magic number if (8 > size) - return {st::cut_magic_number, "Comment header too short for the magic number"}; + throw status {st::cut_magic_number, "Comment header too short for the magic number"}; if (memcmp(data, "OpusTags", 8) != 0) - return {st::bad_magic_number, "Comment header did not start with OpusTags"}; + throw status {st::bad_magic_number, "Comment header did not start with OpusTags"}; // Vendor pos = 8; if (pos + 4 > size) - return {st::cut_vendor_length, + throw status {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::cut_vendor_data, "Vendor string did not fit the comment header"}; + throw status {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::cut_comment_count, "Comment count did not fit the comment header"}; + throw status {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::cut_comment_length, - "Comment length did not fit the comment header"}; + throw status {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::cut_comment_data, - "Comment string did not fit the comment header"}; + throw status {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; @@ -88,8 +88,7 @@ ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags) // Extra data my_tags.extra_data = std::string(data + pos, size - pos); - tags = std::move(my_tags); - return st::ok; + return my_tags; } ot::dynamic_ogg_packet ot::render_tags(const opus_tags& tags) diff --git a/src/opustags.h b/src/opustags.h index 3f7cc86..19551cc 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -376,10 +376,8 @@ struct opus_tags { /** * Read the given OpusTags packet and extract its content into an opus_tags object. - * - * On error, the tags object is left unchanged. */ -status parse_tags(const ogg_packet& packet, opus_tags& tags); +opus_tags parse_tags(const ogg_packet& packet); /** * Serialize an #opus_tags object into an OpusTags Ogg packet. diff --git a/t/opus.cc b/t/opus.cc index 23db8be..f602cb2 100644 --- a/t/opus.cc +++ b/t/opus.cc @@ -14,13 +14,10 @@ static const char standard_OpusTags[] = static void parse_standard() { - ot::opus_tags tags; ogg_packet op; op.bytes = sizeof(standard_OpusTags) - 1; op.packet = (unsigned char*) standard_OpusTags; - auto rc = ot::parse_tags(op, tags); - if (rc != ot::st::ok) - throw failure("ot::parse_tags did not return ok"); + ot::opus_tags tags = ot::parse_tags(op); if (tags.vendor != "opustags test packet") throw failure("bad vendor string"); if (tags.comments.size() != 2) @@ -35,6 +32,16 @@ static void parse_standard() throw failure("found mysterious padding data"); } +static ot::status try_parse_tags(const ogg_packet& packet) +{ + try { + ot::parse_tags(packet); + return ot::st::ok; + } catch (const ot::status& rc) { + return rc; + } +} + /** * Try parse_tags with packets that should not valid, or that might even * corrupt the memory. Run this one with valgrind to ensure we're not @@ -59,43 +66,40 @@ static void parse_corrupted() char* end = packet + size; op.bytes = 7; - if (ot::parse_tags(op, tags) != ot::st::cut_magic_number) + if (try_parse_tags(op) != 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::cut_vendor_length) + if (try_parse_tags(op) != ot::st::cut_vendor_length) throw failure("did not detect the overflowing vendor string length"); op.bytes = size; header_data[0] = 'o'; - if (ot::parse_tags(op, tags) != ot::st::bad_magic_number) + if (try_parse_tags(op) != ot::st::bad_magic_number) throw failure("did not detect the bad magic number"); header_data[0] = 'O'; *vendor_length = end - vendor_string + 1; - if (ot::parse_tags(op, tags) != ot::st::cut_vendor_data) + if (try_parse_tags(op) != 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::cut_comment_count) + if (try_parse_tags(op) != 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::cut_comment_length) + if (try_parse_tags(op) != 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::cut_comment_data) + if (try_parse_tags(op) != ot::st::cut_comment_data) throw failure("did not detect the overflowing comment data"); } static void recode_standard() { - ot::opus_tags tags; ogg_packet op; op.bytes = sizeof(standard_OpusTags) - 1; op.packet = (unsigned char*) standard_OpusTags; - auto rc = ot::parse_tags(op, tags); - if (rc != ot::st::ok) - throw failure("ot::parse_tags did not return ok"); + ot::opus_tags tags = ot::parse_tags(op); auto packet = ot::render_tags(tags); if (packet.b_o_s != 0) throw failure("b_o_s should not be set"); @@ -113,7 +117,6 @@ static void recode_standard() static void recode_padding() { - ot::opus_tags tags; std::string padded_OpusTags(standard_OpusTags, sizeof(standard_OpusTags)); // ^ note: padded_OpusTags ends with a null byte here padded_OpusTags += "hello"; @@ -121,9 +124,7 @@ static void recode_padding() op.bytes = padded_OpusTags.size(); op.packet = (unsigned char*) padded_OpusTags.data(); - auto rc = ot::parse_tags(op, tags); - if (rc != ot::st::ok) - throw failure("ot::parse_tags did not return ok"); + ot::opus_tags tags = ot::parse_tags(op); if (tags.extra_data != "\0hello"s) throw failure("corrupted extra data"); // recode the packet and ensure it's exactly the same diff --git a/t/tap.h b/t/tap.h index 15e5144..5c415bf 100644 --- a/t/tap.h +++ b/t/tap.h @@ -30,6 +30,8 @@ static void run(F test, const char *name) ok = true; } catch (failure& e) { std::cerr << "# fail: " << e.what() << "\n"; + } catch (const ot::status &rc) { + std::cerr << "# unexpected error: " << rc.message << "\n"; } std::cout << (ok ? "ok" : "not ok") << " - " << name << "\n"; }