release 1.4.0

Co-authored-by: Frédéric Mangano-Tarumi <fmang@mg0.fr>

CHANGELOG.md

@@ -1,6 +1,16 @@
 opustags changelog
 ==================
 
+1.4.0 - 2020-10-04
+------------------
+
+- Preserve permissions when overwriting files.
+- Support multiple files with --in-place.
+- Fix BSD support.
+
+Thanks to Reuben Thomas for contributing the pièce de résistance of this
+release!
+
 1.3.0 - 2019-02-02
 ------------------
 

CMakeLists.txt

@@ -2,13 +2,18 @@ cmake_minimum_required(VERSION 3.9)
 
 project(
 	opustags
-	VERSION 1.3.0
+	VERSION 1.4.0
 	LANGUAGES CXX
 )
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
+# opustags is mainly developed with glibc, which introduces a few
+# incompatibilites with BSDs, like getline not being defined by default.
+# _GNU_SOURCE should trigger BSD’s libc GNU compatibility mode to fix that.
+add_definitions(-D_GNU_SOURCE)
+
 find_package(PkgConfig REQUIRED)
 pkg_check_modules(OGG REQUIRED ogg)
 add_compile_options(${OGG_CFLAGS})

CONTRIBUTING.md

@@ -42,7 +42,7 @@ questioned, and it was thus abandoned for a few years. Judging by the
 inquiries and contributions, albeit few, on GitHub, it looks like it remains
 relevant, so let's dust it off a bit.
 
-Today, opustags is written in C++14 and features a unit test suite in C++, and
+Today, opustags is written in C++ and features a unit test suite in C++, and
 an integration test suite in Perl. The code was refactored, organized into
 modules, and reviewed for safety.
 

README.md

@@ -21,7 +21,7 @@ Requirements
 ------------
 
 * a POSIX-compliant system,
-* a C++14 compiler,
+* a C++17 compiler,
 * CMake ≥ 3.9,
 * libogg 1.3.3.
 
@@ -48,12 +48,13 @@ Documentation
 
     Usage: opustags --help
            opustags [OPTIONS] FILE
+           opustags OPTIONS -i FILE...
            opustags OPTIONS FILE -o FILE
 
     Options:
       -h, --help                    print this help
       -o, --output FILE             specify the output file
-      -i, --in-place                overwrite the input file
+      -i, --in-place                overwrite the input files
       -y, --overwrite               overwrite the output file if it already exists
       -a, --add FIELD=VALUE         add a comment
       -d, --delete FIELD[=VALUE]    delete previously existing comments

opustags.1

@@ -10,6 +10,11 @@ opustags \- Ogg Opus tag editor
 .br
 .B opustags
 .I OPTIONS
+.B -i
+.R \fIFILE\fP...
+.br
+.B opustags
+.I OPTIONS
 .B -o
 .I OUTPUT INPUT
 .SH DESCRIPTION
@@ -24,7 +29,7 @@ You can use the options below to edit the tags before printing them.
 This could be useful to preview some changes before writing them.
 .PP
 In editing mode, you need to specify an output file with \fB--output\fP, or use \fB--in-place\fP to
-overwrite the input file. If the output is a regular file, the result is first written to a
+overwrite the input files. If the output is a regular file, the result is first written to a
 temporary file and then moved to its final location on success. On error, the temporary output file
 is deleted.
 .PP

src/cli.cc

@@ -4,9 +4,6 @@
  *
  * Provide all the features of the opustags executable from a C++ API. The main point of separating
  * this module from the main one is to allow easy testing.
- *
- * \todo Use a safer temporary file name for in-place editing, like tmpnam.
- * \todo Abort editing with --set-all if one comment is invalid?
  */
 
 #include <config.h>
@@ -26,12 +23,13 @@ R"raw(
 
 Usage: opustags --help
        opustags [OPTIONS] FILE
+       opustags OPTIONS -i FILE...
        opustags OPTIONS FILE -o FILE
 
 Options:
   -h, --help                    print this help
   -o, --output FILE             specify the output file
-  -i, --in-place                overwrite the input file
+  -i, --in-place                overwrite the input files
   -y, --overwrite               overwrite the output file if it already exists
   -a, --add FIELD=VALUE         add a comment
   -d, --delete FIELD[=VALUE]    delete previously existing comments
@@ -55,16 +53,16 @@ static struct option getopt_options[] = {
 	{NULL, 0, 0, 0}
 };
 
-ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
+ot::status ot::parse_options(int argc, char** argv, ot::options& opt, FILE* comments_input)
 {
 	static ot::encoding_converter to_utf8("", "UTF-8");
 	std::string utf8;
 	std::string::size_type equal;
 	ot::status rc;
+	bool set_all = false;
 	opt = {};
 	if (argc == 1)
 		return {st::bad_arguments, "No arguments specified. Use -h for help."};
-	bool in_place = false;
 	int c;
 	optind = 0;
 	while ((c = getopt_long(argc, argv, ":ho:iyd:a:s:DS", getopt_options, NULL)) != -1) {
@@ -73,14 +71,12 @@ ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
 			opt.print_help = true;
 			break;
 		case 'o':
-			if (!opt.path_out.empty())
+			if (opt.path_out)
 				return {st::bad_arguments, "Cannot specify --output more than once."};
 			opt.path_out = optarg;
-			if (opt.path_out.empty())
-				return {st::bad_arguments, "Output file path cannot be empty."};
 			break;
 		case 'i':
-			in_place = true;
+			opt.in_place = true;
 			break;
 		case 'y':
 			opt.overwrite = true;
@@ -103,7 +99,8 @@ ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
 			opt.to_add.emplace_back(std::move(utf8));
 			break;
 		case 'S':
-			opt.set_all = true;
+			opt.delete_all = true;
+			set_all = true;
 			break;
 		case 'D':
 			opt.delete_all = true;
@@ -118,27 +115,42 @@ ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
 	}
 	if (opt.print_help)
 		return st::ok;
-	if (optind != argc - 1)
-		return {st::bad_arguments, "Exactly one input file must be specified."};
-	opt.path_in = argv[optind];
-	if (opt.path_in.empty())
-		return {st::bad_arguments, "Input file path cannot be empty."};
-	if (in_place) {
-		if (!opt.path_out.empty())
+	if (opt.in_place) {
+		if (opt.path_out)
 			return {st::bad_arguments, "Cannot combine --in-place and --output."};
-		if (opt.path_in == "-")
-			return {st::bad_arguments, "Cannot modify standard input in place."};
-		opt.path_out = opt.path_in;
 		opt.overwrite = true;
+		for (int i = optind; i < argc; i++) {
+			if (strcmp(argv[i], "-") == 0)
+				return {st::bad_arguments, "Cannot modify standard input in place."};
+			opt.paths_in.emplace_back(argv[i]);
+		}
+	} else {
+		if (optind != argc - 1)
+			return {st::bad_arguments, "Exactly one input file must be specified."};
+		if (set_all && strcmp(argv[optind], "-") == 0)
+			return {st::bad_arguments,
+				"Cannot use standard input as input file when --set-all is specified."};
+		opt.paths_in.emplace_back(argv[optind]);
+	}
+	if (set_all) {
+		// Read comments from stdin and prepend them to opt.to_add.
+		std::vector<std::string> comments;
+		auto rc = read_comments(comments_input, comments);
+		if (rc != st::ok)
+			return rc;
+		comments.reserve(comments.size() + opt.to_add.size());
+		std::move(opt.to_add.begin(), opt.to_add.end(), std::back_inserter(comments));
+		opt.to_add = std::move(comments);
 	}
-	if (opt.path_in == "-" && opt.set_all)
-		return {st::bad_arguments,
-		        "Cannot use standard input as input file when --set-all is specified."};
 	return st::ok;
 }
 
 /**
- * \todo Escape new lines.
+ * \todo Find a way to support new lines such that they can be read back by #read_comment without
+ *       ambiguity. We could add a raw mode and separate comments with a \0, or escape control
+ *       characters with a backslash, but we should also preserve compatibiltity with potential
+ *       callers that don’t escape backslashes. Maybe add options to select a mode between simple,
+ *       raw, and escaped.
  */
 void ot::print_comments(const std::list<std::string>& comments, FILE* output)
 {
@@ -176,7 +188,7 @@ void ot::print_comments(const std::list<std::string>& comments, FILE* output)
 		fputs("warning: Some tags contain control characters.\n", stderr);
 }
 
-ot::status ot::read_comments(FILE* input, std::list<std::string>& comments)
+ot::status ot::read_comments(FILE* input, std::vector<std::string>& comments)
 {
 	static ot::encoding_converter to_utf8("", "UTF-8");
 	comments.clear();
@@ -232,11 +244,7 @@ void ot::delete_comments(std::list<std::string>& comments, const std::string& se
 /** Apply the modifications requested by the user to the opustags packet. */
 static ot::status edit_tags(ot::opus_tags& tags, const ot::options& opt)
 {
-	if (opt.set_all) {
-		auto rc = ot::read_comments(stdin, tags.comments);
-		if (rc != ot::st::ok)
-			return rc;
-	} else if (opt.delete_all) {
+	if (opt.delete_all) {
 		tags.comments.clear();
 	} else for (const std::string& name : opt.to_delete) {
 		ot::delete_comments(tags.comments, name.c_str());
@@ -258,7 +266,6 @@ static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const
 {
 	bool focused = false; /*< the stream on which we operate is defined */
 	int focused_serialno; /*< when focused, the serialno of the focused stream */
-	/** \todo Become stream-aware instead of counting the pages of all streams together. */
 	int absolute_page_no = -1; /*< page number in the physical stream, not logical */
 	for (;;) {
 		ot::status rc = reader.next_page();
@@ -275,6 +282,7 @@ static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const
 			focused = true;
 			focused_serialno = serialno;
 		} else if (serialno != focused_serialno) {
+			/** \todo Support mixed streams. */
 			return {ot::st::error, "Muxed streams are not supported yet."};
 		}
 		if (absolute_page_no == 0) { // Identification header
@@ -312,23 +320,18 @@ static ot::status process(ot::ogg_reader& reader, ot::ogg_writer* writer, const
 	return ot::st::ok;
 }
 
-ot::status ot::run(const ot::options& opt)
+static ot::status run_single(const ot::options& opt, const std::string& path_in, const std::optional<std::string>& path_out)
 {
-	if (opt.print_help) {
-		fputs(help_message, stdout);
-		return st::ok;
-	}
-
 	ot::file input;
-	if (opt.path_in == "-")
+	if (path_in == "-")
 		input = stdin;
-	else if ((input = fopen(opt.path_in.c_str(), "r")) == nullptr)
+	else if ((input = fopen(path_in.c_str(), "r")) == nullptr)
 		return {ot::st::standard_error,
-		        "Could not open '" + opt.path_in + "' for reading: " + strerror(errno)};
+		        "Could not open '" + path_in + "' for reading: " + strerror(errno)};
 	ot::ogg_reader reader(input.get());
 
 	/* Read-only mode. */
-	if (opt.path_out.empty())
+	if (!path_out)
 		return process(reader, nullptr, opt);
 
 	/* Read-write mode.
@@ -339,11 +342,10 @@ ot::status ot::run(const ot::options& opt)
 	 *  - temporary_output.get() for regular files.
 	 *
 	 * We use a temporary output file for the following reasons:
-	 *  1. The partial .opus output may be seen by softwares like media players, or through
-	 *     inotify for the most attentive process.
+	 *  1. A partial .opus output would be seen by softwares like media players, but a .part
+	 *     (for partial) won’t.
 	 *  2. If the process crashes badly, or the power cuts off, we don't want to leave a partial
-	 *     file at the final location. The temporary file is still going to stay but will have an
-	 *     obvious name.
+	 *     file at the final location. The temporary file is going to remain though.
 	 *  3. If we're overwriting a regular file, we'd rather avoid wiping its content before we
 	 *     even started reading the input file. That way, the original file is always preserved
 	 *     on error or crash.
@@ -357,30 +359,30 @@ ot::status ot::run(const ot::options& opt)
 
 	ot::status rc = ot::st::ok;
 	struct stat output_info;
-	if (opt.path_out == "-") {
+	if (path_out == "-") {
 		output = stdout;
-	} else if (stat(opt.path_out.c_str(), &output_info) == 0) {
+	} else if (stat(path_out->c_str(), &output_info) == 0) {
 		/* The output file exists. */
 		if (!S_ISREG(output_info.st_mode)) {
 			/* Special files are opened for writing directly. */
-			if ((final_output = fopen(opt.path_out.c_str(), "w")) == nullptr)
+			if ((final_output = fopen(path_out->c_str(), "w")) == nullptr)
 				rc = {ot::st::standard_error,
-				      "Could not open '" + opt.path_out + "' for writing: " +
-				       strerror(errno)};
+				      "Could not open '" + path_out.value() + "' for writing: " +
+				      strerror(errno)};
 			output = final_output.get();
 		} else if (opt.overwrite) {
-			rc = temporary_output.open(opt.path_out.c_str());
+			rc = temporary_output.open(path_out->c_str());
 			output = temporary_output.get();
 		} else {
 			rc = {ot::st::error,
-			      "'" + opt.path_out + "' already exists. Use -y to overwrite."};
+			      "'" + path_out.value() + "' already exists. Use -y to overwrite."};
 		}
 	} else if (errno == ENOENT) {
-		rc = temporary_output.open(opt.path_out.c_str());
+		rc = temporary_output.open(path_out->c_str());
 		output = temporary_output.get();
 	} else {
 		rc = {ot::st::error,
-		      "Could not identify '" + opt.path_in + "': " + strerror(errno)};
+		      "Could not identify '" + path_in + "': " + strerror(errno)};
 	}
 	if (rc != ot::st::ok)
 		return rc;
@@ -392,3 +394,22 @@ ot::status ot::run(const ot::options& opt)
 
 	return rc;
 }
+
+ot::status ot::run(const ot::options& opt)
+{
+	if (opt.print_help) {
+		fputs(help_message, stdout);
+		return st::ok;
+	}
+
+	ot::status global_rc = st::ok;
+	for (const auto& path_in : opt.paths_in) {
+		ot::status rc = run_single(opt, path_in, opt.in_place ? path_in : opt.path_out);
+		if (rc != st::ok) {
+			global_rc = st::error;
+			if (!rc.message.empty())
+				fprintf(stderr, "%s: error: %s\n", path_in.c_str(), rc.message.c_str());
+		}
+	}
+	return global_rc;
+}

src/opus.cc

@@ -18,7 +18,6 @@
  *
  * \todo Validate that the vendor string and comments are valid UTF-8.
  * \todo Validate that field names are ASCII: 0x20 through 0x7D, 0x3D ('=') excluded.
- * \todo Field names are case insensitive, respect that.
  *
  */
 
@@ -32,9 +31,6 @@
 #define le32toh(x) OSSwapLittleToHostInt32(x)
 #endif
 
-/**
- * \todo See if the packet's data could be casted more nicely into a string.
- */
 ot::status ot::parse_tags(const ogg_packet& packet, opus_tags& tags)
 {
 	if (packet.bytes < 0)

src/opustags.cc

@@ -17,15 +17,11 @@
 int main(int argc, char** argv) {
 	setlocale(LC_ALL, "");
 	ot::options opt;
-	ot::status rc = ot::parse_options(argc, argv, opt);
+	ot::status rc = ot::parse_options(argc, argv, opt, stdin);
 	if (rc == ot::st::ok)
 		rc = ot::run(opt);
+	else if (!rc.message.empty())
+		fprintf(stderr, "error: %s\n", rc.message.c_str());
 
-	if (rc != ot::st::ok) {
-		if (!rc.message.empty())
-			fprintf(stderr, "error: %s\n", rc.message.c_str());
-		return EXIT_FAILURE;
-	} else {
-		return EXIT_SUCCESS;
-	}
+	return rc == ot::st::ok ? EXIT_SUCCESS : EXIT_FAILURE;
 }

src/opustags.h

@@ -374,18 +374,20 @@ struct options {
 	 */
 	bool print_help = false;
 	/**
-	 * Path to the input file. It cannot be empty. The special "-" string means stdin.
+	 * Paths to the input files. The special string "-" means stdin.
 	 *
-	 * This is the mandatory non-flagged parameter.
+	 * At least one input file must be given. If `--in-place` is used,
+	 * more than one may be given.
 	 */
-	std::string path_in;
+	std::vector<std::string> paths_in;
 	/**
-	 * Path to the optional file. The special "-" string means stdout. When empty, opustags runs
-	 * in read-only mode. For in-place editing, path_out is defined equal to path_in.
+	 * Optional path to output file. The special string "-" means stdout. For in-place
+	 * editing, the input file name is used. If no output file name is supplied, and
+	 * --in-place is not used, opustags runs in read-only mode.
 	 *
 	 * Options: --output, --in-place
 	 */
-	std::string path_out;
+	std::optional<std::string> path_out;
 	/**
 	 * By default, opustags won't overwrite the output file if it already exists. This can be
 	 * forced with --overwrite. It is also enabled by --in-place.
@@ -393,6 +395,12 @@ struct options {
 	 * Options: --overwrite, --in-place
 	 */
 	bool overwrite = false;
+	/**
+	 * Process files in-place.
+	 *
+	 * Options: --in-place
+	 */
+	bool in_place = false;
 	/**
 	 * List of comments to delete. Each string is a selector according to the definition of
 	 * #delete_comments.
@@ -410,7 +418,7 @@ struct options {
 	/**
 	 * Delete all the existing comments.
 	 *
-	 * Option: --delete-all
+	 * Option: --delete-all, --set-all
 	 */
 	bool delete_all = false;
 	/**
@@ -422,24 +430,15 @@ struct options {
 	 * Options: --add, --set, --set-all
 	 */
 	std::vector<std::string> to_add;
-	/**
-	 * Replace the previous comments by the ones supplied by the user.
-	 *
-	 * Read a list of comments from stdin and populate #to_add. Further comments may be added
-	 * with the --add option.
-	 *
-	 * Option: --set-all
-	 */
-	bool set_all = false;
 };
 
 /**
  * Parse the command-line arguments. Does not perform I/O related validations, but checks the
- * consistency of its arguments.
+ * consistency of its arguments. Comments are read if necessary from the given stream.
  *
  * On error, the state of the options structure is unspecified.
  */
-status parse_options(int argc, char** argv, options& opt);
+status parse_options(int argc, char** argv, options& opt, FILE* comments);
 
 /**
  * Print all the comments, separated by line breaks. Since a comment may contain line breaks, this
@@ -447,7 +446,7 @@ status parse_options(int argc, char** argv, options& opt);
  *
  * The comments must be encoded in UTF-8, and are converted to the system locale when printed.
  *
- * The output generated is meant to be parseable by #ot::read_tags.
+ * The output generated is meant to be parseable by #ot::read_comments.
  */
 void print_comments(const std::list<std::string>& comments, FILE* output);
 
@@ -456,7 +455,7 @@ void print_comments(const std::list<std::string>& comments, FILE* output);
  *
  * The comments are converted from the system encoding to UTF-8, and returned as UTF-8.
  */
-status read_comments(FILE* input, std::list<std::string>& comments);
+status read_comments(FILE* input, std::vector<std::string>& comments);
 
 /**
  * Remove all comments matching the specified selector, which may either be a field name or a

src/system.cc

@@ -13,6 +13,7 @@
 
 #include <errno.h>
 #include <string.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
 ot::status ot::partial_file::open(const char* destination)
@@ -33,11 +34,45 @@ ot::status ot::partial_file::open(const char* destination)
 	return st::ok;
 }
 
+static mode_t get_umask()
+{
+	// libc doesn’t seem to provide a way to get umask without changing it, so we need this workaround.
+	// https://www.gnu.org/software/libc/manual/html_node/Setting-Permissions.html
+	mode_t mask = umask(0);
+	umask(mask);
+	return mask;
+}
+
+/**
+ * Try reproducing the file permissions of file `source` onto file `dest`. If
+ * this fails for whatever reason, print a warning and leave the current
+ * permissions. When the source doesn’t exist, use the default file creation
+ * permissions according to umask.
+ */
+static void copy_permissions(const char* source, const char* dest)
+{
+	mode_t target_mode;
+	struct stat source_stat;
+	if (stat(source, &source_stat) == 0) {
+		// We could technically preserve a bit more than that but who
+		// would ever need S_ISUID and friends on an Opus file?
+		target_mode = source_stat.st_mode & 0777;
+	} else if (errno == ENOENT) {
+		target_mode = 0666 & ~get_umask();
+	} else {
+		fprintf(stderr, "warning: Could not read mode of %s: %s\n", source, strerror(errno));
+		return;
+	}
+	if (chmod(dest, target_mode) == -1)
+		fprintf(stderr, "warning: Could not set mode of %s: %s\n", dest, strerror(errno));
+}
+
 ot::status ot::partial_file::commit()
 {
 	if (file == nullptr)
 		return st::ok;
 	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 '" +

t/cli.cc

@@ -7,7 +7,7 @@ using namespace std::literals::string_literals;
 
 void check_read_comments()
 {
-	std::list<std::string> comments;
+	std::vector<std::string> comments;
 	ot::status rc;
 	{
 		std::string txt = "TITLE=a b c\n\nARTIST=X\nArtist=Y\n"s;
@@ -39,14 +39,14 @@ void check_read_comments()
  * Wrap #ot::parse_options with a higher-level interface much more convenient for testing.
  * In practice, the argc/argv combo are enough though for the current state of opustags.
  */
-static ot::status parse_options(const std::vector<const char*>& args, ot::options& opt)
+static ot::status parse_options(const std::vector<const char*>& args, ot::options& opt, FILE *comments)
 {
 	int argc = args.size();
 	char* argv[argc];
-	for (size_t i = 0; i < argc; ++i)
+	for (int i = 0; i < argc; ++i)
 		argv[i] = strdup(args[i]);
-	ot::status rc = ot::parse_options(argc, argv, opt);
-	for (size_t i = 0; i < argc; ++i)
+	ot::status rc = ot::parse_options(argc, argv, opt, comments);
+	for (int i = 0; i < argc; ++i)
 		free(argv[i]);
 	return rc;
 }
@@ -55,7 +55,9 @@ void check_good_arguments()
 {
 	auto parse = [](std::vector<const char*> args) {
 		ot::options opt;
-		ot::status rc = parse_options(args, opt);
+		std::string txt = "N=1\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		ot::status rc = parse_options(args, opt, input.get());
 		if (rc.code != ot::st::ok)
 			throw failure("unexpected option parsing error");
 		return opt;
@@ -67,29 +69,40 @@ void check_good_arguments()
 		throw failure("did not catch --help");
 
 	opt = parse({"opustags", "x", "--output", "y", "-D", "-s", "X=Y Z", "-d", "a=b"});
-	if (opt.path_in != "x" || opt.path_out != "y" || !opt.delete_all || opt.overwrite ||
-	    opt.to_delete.size() != 2 || opt.to_delete[0] != "X" || opt.to_delete[1] != "a=b" ||
+	if (opt.paths_in.size() != 1 || opt.paths_in.front() != "x" || !opt.path_out ||
+	    opt.path_out != "y" || !opt.delete_all || opt.overwrite || opt.to_delete.size() != 2 ||
+	    opt.to_delete[0] != "X" || opt.to_delete[1] != "a=b" ||
 	    opt.to_add.size() != 1 || opt.to_add[0] != "X=Y Z")
 		throw failure("unexpected option parsing result for case #1");
 
 	opt = parse({"opustags", "-S", "x", "-S", "-a", "x=y z", "-i"});
-	if (opt.path_in != "x" || opt.path_out != "x" || !opt.set_all || !opt.overwrite ||
-	    opt.to_delete.size() != 0 || opt.to_add.size() != 1 || opt.to_add[0] != "x=y z")
+	if (opt.paths_in.size() != 1 || opt.paths_in.front() != "x" || opt.path_out ||
+	    !opt.overwrite || opt.to_delete.size() != 0 ||
+	    opt.to_add.size() != 2 || opt.to_add[0] != "N=1" || opt.to_add[1] != "x=y z")
 		throw failure("unexpected option parsing result for case #2");
+
+	opt = parse({"opustags", "-i", "x", "y", "z"});
+	if (opt.paths_in.size() != 3 || opt.paths_in[0] != "x" || opt.paths_in[1] != "y" ||
+	    opt.paths_in[2] != "z" || !opt.overwrite || !opt.in_place)
+		throw failure("unexpected option parsing result for case #3");
 }
 
 void check_bad_arguments()
 {
-	auto error_case = [](std::vector<const char*> args, const char* message, const std::string& name) {
+	auto error_code_case = [](std::vector<const char*> args, const char* message, ot::st error_code, const std::string& name) {
 		ot::options opt;
-		ot::status rc = parse_options(args, opt);
-		if (rc.code != ot::st::bad_arguments)
+		std::string txt = "N=1\nINVALID"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		ot::status rc = parse_options(args, opt, input.get());
+		if (rc.code != error_code)
 			throw failure("bad error code for case " + name);
 		if (rc.message != message)
 			throw failure("bad error message for case " + name + ", got: " + rc.message);
 	};
+	auto error_case = [&error_code_case](std::vector<const char*> args, const char* message, const std::string& name) {
+		 error_code_case(args, message, ot::st::bad_arguments, name);
+	};
 	error_case({"opustags"}, "No arguments specified. Use -h for help.", "no arguments");
-	error_case({"opustags", "--output", ""}, "Output file path cannot be empty.", "empty output path");
 	error_case({"opustags", "-a", "X"}, "Comment does not contain an equal sign: X.", "bad comment for -a");
 	error_case({"opustags", "--set", "X"}, "Comment does not contain an equal sign: X.", "bad comment for --set");
 	error_case({"opustags", "-a"}, "Missing value for option '-a'.", "short option with missing value");
@@ -99,13 +112,15 @@ void check_bad_arguments()
 	error_case({"opustags", "-x=y"}, "Unrecognized option '-x'.", "unrecognized short option with value");
 	error_case({"opustags", "--derp=y"}, "Unrecognized option '--derp=y'.", "unrecognized long option with value");
 	error_case({"opustags", "-aX=Y"}, "Exactly one input file must be specified.", "no input file");
-	error_case({"opustags", ""}, "Input file path cannot be empty.", "empty input file path");
 	error_case({"opustags", "-i", "-o", "/dev/null", "-"}, "Cannot combine --in-place and --output.", "in-place + output");
 	error_case({"opustags", "-S", "-"}, "Cannot use standard input as input file when --set-all is specified.",
 	                                    "set all and read opus from stdin");
 	error_case({"opustags", "-i", "-"}, "Cannot modify standard input in place.", "write stdin in-place");
 	error_case({"opustags", "-o", "x", "--output", "y", "z"},
 	           "Cannot specify --output more than once.", "double output");
+	error_code_case({"opustags", "-S", "x"}, "Malformed tag: INVALID", ot::st::error, "attempt to read invalid argument with -S");
+	error_case({"opustags", "-o", "", "--output", "y", "z"},
+	           "Cannot specify --output more than once.", "double output with first filename empty");
 }
 
 static void check_delete_comments()

t/opustags.t

@@ -4,10 +4,11 @@ use strict;
 use warnings;
 use utf8;
 
-use Test::More tests => 34;
+use Test::More tests => 41;
 
 use Digest::MD5;
 use File::Basename;
+use File::Copy;
 use IPC::Open3;
 use List::MoreUtils qw(any);
 use Symbol 'gensym';
@@ -57,12 +58,13 @@ $version
 
 Usage: opustags --help
        opustags [OPTIONS] FILE
+       opustags OPTIONS -i FILE...
        opustags OPTIONS FILE -o FILE
 
 Options:
   -h, --help                    print this help
   -o, --output FILE             specify the output file
-  -i, --in-place                overwrite the input file
+  -i, --in-place                overwrite the input files
   -y, --overwrite               overwrite the output file if it already exists
   -a, --add FIELD=VALUE         add a comment
   -d, --delete FIELD[=VALUE]    delete previously existing comments
@@ -81,7 +83,7 @@ error: Unrecognized option '--derp'.
 EOF
 
 is_deeply(opustags('../opustags'), ['', <<"EOF", 256], 'not an Ogg stream');
-error: Input is not a valid Ogg file.
+../opustags: error: Input is not a valid Ogg file.
 EOF
 
 ####################################################################################################
@@ -101,25 +103,32 @@ encoder=Lavc58.18.100 libopus
 EOF
 
 unlink('out.opus');
+my $previous_umask = umask(0022);
 is_deeply(opustags(qw(gobble.opus -o out.opus)), ['', '', 0], 'copy the file without changes');
 is(md5('out.opus'), '111a483596ac32352fbce4d14d16abd2', 'the copy is faithful');
+is((stat 'out.opus')[2] & 0777, 0644, 'apply umask on new files');
+umask($previous_umask);
 
 # empty out.opus
 { my $fh; open($fh, '>', 'out.opus') and close($fh) or die }
 is_deeply(opustags(qw(gobble.opus -o out.opus)), ['', <<'EOF', 256], 'refuse to override');
-error: 'out.opus' already exists. Use -y to overwrite.
+gobble.opus: error: 'out.opus' already exists. Use -y to overwrite.
 EOF
 is(md5('out.opus'), 'd41d8cd98f00b204e9800998ecf8427e', 'the output wasn\'t written');
 
 is_deeply(opustags(qw(gobble.opus -o /dev/null)), ['', '', 0], 'write to /dev/null');
 
+chmod(0604, 'out.opus');
 is_deeply(opustags(qw(gobble.opus -o out.opus --overwrite)), ['', '', 0], 'overwrite');
 is(md5('out.opus'), '111a483596ac32352fbce4d14d16abd2', 'successfully overwritten');
+is((stat 'out.opus')[2] & 0777, 0604, 'overwriting preserves output file\'s mode');
 
+chmod(0700, 'out.opus');
 is_deeply(opustags(qw(--in-place out.opus -a A=B --add=A=C --add), "TITLE=Foo Bar",
                    qw(--delete A --add TITLE=七面鳥 --set encoder=whatever -s 1=2 -s X=1 -a X=2 -s X=3)),
           ['', '', 0], 'complex tag editing');
 is(md5('out.opus'), '66780307a6081523dc9040f3c47b0448', 'check the footprint');
+is((stat 'out.opus')[2] & 0777, 0700, 'in-place editing preserves file mode');
 
 is_deeply(opustags('out.opus'), [<<'EOF', '', 0], 'check the tags written');
 A=B
@@ -201,6 +210,17 @@ is_deeply(opustags('-', '-o', '-', {in => $data, mode => ':raw'}), [$data, '', 0
 
 unlink('out.opus');
 
+# Test --in-place
+unlink('out2.opus');
+copy('gobble.opus', 'out.opus');
+is_deeply(opustags(qw(out.opus --add BAR=baz -o out2.opus)), ['', '', 0], 'process multiple files with --in-place');
+is_deeply(opustags(qw(--in-place --add FOO=bar out.opus out2.opus)), ['', '', 0], 'process multiple files with --in-place');
+is(md5('out.opus'), '30ba30c4f236c09429473f36f8f861d2', 'the tags were added correctly (out.opus)');
+is(md5('out2.opus'), '0a4d20c287b2e46b26cb0eee353c2069', 'the tags were added correctly (out2.opus)');
+
+unlink('out.opus');
+unlink('out2.opus');
+
 ####################################################################################################
 # Test muxed streams
 
@@ -208,7 +228,7 @@ system('ffmpeg -loglevel error -y -i gobble.opus -c copy -map 0:0 -map 0:0 -shor
 	or BAIL_OUT('could not create a muxed stream');
 
 is_deeply(opustags('muxed.ogg'), ['', <<'END_ERR', 256], 'muxed streams detection');
-error: Muxed streams are not supported yet.
+muxed.ogg: error: Muxed streams are not supported yet.
 END_ERR
 
 unlink('muxed.ogg');