Expand EDITOR/VISUAL with wordexp

opustags.1

@@ -103,6 +103,7 @@ Blank lines and lines starting with \fI#\fP are ignored.
 Edit tags interactively by spawning the program specified by the EDITOR
 environment variable. The allowed format is the same as \fB--set-all\fP.
 If TERM and VISUAL are set, VISUAL takes precedence over EDITOR.
+Both variables are expanded with wordexp(3).
 .SH EXAMPLES
 .PP
 List all the tags in file foo.opus:

src/cli.cc

@@ -298,7 +298,7 @@ static ot::status edit_tags_interactively(ot::opus_tags& tags, const std::option
 	if (fclose(tags_file) != 0)
 		return {ot::st::standard_error, "fclose error: "s + strerror(errno)};
 
-	ot::status rc = ot::execute_process(editor, tags_path);
+	ot::status rc = ot::run_editor(editor, tags_path.c_str());
 	if (rc != ot::st::ok)
 		return rc;
 

src/opustags.h

@@ -163,10 +163,14 @@ private:
 	iconv_t cd; /**< conversion descriptor */
 };
 
-// Execute the process specified in args using execlp. Wait for the process to
-// exit and return st::ok on success, or st::child_process_failed if it did not
-// exit with 0.
-ot::status execute_process(std::string_view arg0, std::string_view arg1);
+/**
+ * Execute the editor process specified in editor using execlp. Wait for the process to exit and
+ * return st::ok on success, or st::child_process_failed if it did not exit with 0.
+ *
+ * editor may contain options, which will be expanded using wordexp(3).
+ * path is the name of the file to edit, which will be passed as the last argument to editor.
+ */
+ot::status run_editor(const char* editor, const char* path);
 
 /** \} */
 

src/system.cc

@@ -16,6 +16,7 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include <wordexp.h>
 
 using namespace std::string_literals;
 
@@ -136,15 +137,31 @@ ot::status ot::encoding_converter::operator()(const char* in, size_t n, std::str
 	return ot::st::ok;
 }
 
-ot::status ot::execute_process(std::string_view arg0, std::string_view arg1)
+ot::status ot::run_editor(const char* editor, const char* path)
 {
 	pid_t pid = fork();
 	if (pid == -1) {
 		return {st::standard_error, "Could not fork: "s + strerror(errno)};
 	} else if (pid == 0) {
-		execlp(arg0.data(), arg0.data(), arg1.data(), nullptr);
+		wordexp_t p;
+		if (wordexp(editor, &p, WRDE_SHOWERR) != 0) {
+			fprintf(stderr, "error: wordexp failed while expanding %s\n", editor);
+			exit(1);
+		}
+		// After expansion of editor into an array of words by wordexp, append the file path
+		// and a sentinel nullptr for execvp.
+		std::vector<char*> argv;
+		argv.reserve(p.we_wordc + 2);
+		for (size_t i = 0; i < p.we_wordc; ++i)
+			argv.push_back(p.we_wordv[i]);
+		std::string path_copy = path; // execvp wants a char* and not a const char*
+		argv.push_back(path_copy.data());
+		argv.push_back(nullptr);
+
+		execvp(argv[0], argv.data());
 		// execvp only returns on error. Let’s not have a runaway child process and kill it.
-		fprintf(stderr, "error: execvp failed: %s\n", strerror(errno));
+		fprintf(stderr, "error: execvp %s failed: %s\n", argv[0], strerror(errno));
+		wordfree(&p);
 		exit(1);
 	}
 

t/CMakeLists.txt

@@ -14,7 +14,6 @@ add_executable(oggdump EXCLUDE_FROM_ALL oggdump.cc)
 target_link_libraries(oggdump ot)
 
 configure_file(gobble.opus . COPYONLY)
-configure_file(screamer . COPYONLY)
 configure_file(emptier . COPYONLY)
 
 add_custom_target(

t/opustags.t

@@ -226,7 +226,7 @@ unlink('out2.opus');
 ####################################################################################################
 # Interactive edition
 
-$ENV{EDITOR} = './screamer';
+$ENV{EDITOR} = 'sed -i -e y/a/A/';
 is_deeply(opustags(qw(gobble.opus --add artist=aaah -o screaming.opus -e)), ['', '', 0], 'edit a file with EDITOR');
 is(md5('screaming.opus'), '682229df1df6b0ca147e2778737d449e', 'the tags were modified');
 

t/screamer

@@ -1,2 +0,0 @@
-#!/bin/sh
-exec sed -i -e 'y/a/A/' "$1"