From 8a54361b8fdb643d4b8d3fb9d7545c45acda86f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Mangano?= <fmang@mg0.fr>
Date: Sun, 17 Jan 2021 15:07:56 +0100
Subject: [PATCH] Migrate the opus module to use exceptions

---
 src/cli.cc     |  8 +-------
 src/opus.cc    | 25 ++++++++++++-------------
 src/opustags.h |  4 +---
 t/opus.cc      | 39 ++++++++++++++++++++-------------------
 t/tap.h        |  2 ++
 5 files changed, 36 insertions(+), 42 deletions(-)

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<size_t>(packet.bytes);
 	const char* data = reinterpret_cast<char*>(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";
 }