diff --git a/opustags.1 b/opustags.1 index ef6791e..72ab0b3 100644 --- a/opustags.1 +++ b/opustags.1 @@ -103,7 +103,6 @@ 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: diff --git a/src/cli.cc b/src/cli.cc index e4d8227..3a1abec 100644 --- a/src/cli.cc +++ b/src/cli.cc @@ -302,7 +302,7 @@ static ot::status edit_tags_interactively(ot::opus_tags& tags, const std::option timespec before, after; if ((rc = ot::get_file_timestamp(tags_path.c_str(), before)) != ot::st::ok) return rc; - ot::status editor_rc = ot::run_editor(editor, tags_path.c_str()); + ot::status editor_rc = ot::run_editor(editor, tags_path); if ((rc = ot::get_file_timestamp(tags_path.c_str(), after)) != ot::st::ok) return rc; // probably because the file was deleted bool modified = (before.tv_sec != after.tv_sec || before.tv_nsec != after.tv_nsec); diff --git a/src/opustags.h b/src/opustags.h index 57f74e1..0a909a2 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -170,13 +170,13 @@ private: std::string shell_escape(std::string_view word); /** - * Execute the editor process specified in editor using execlp. Wait for the process to exit and + * Execute the editor process specified in editor. 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). + * editor is passed unescaped to the shell, and may contain CLI options. * 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); +ot::status run_editor(std::string_view editor, std::string_view path); /** * Return the specified path’s mtime, i.e. the last data modification diff --git a/src/system.cc b/src/system.cc index 104d18d..7357a03 100644 --- a/src/system.cc +++ b/src/system.cc @@ -12,11 +12,11 @@ #include #include +#include #include #include #include #include -#include using namespace std::string_literals; @@ -157,36 +157,12 @@ std::string ot::shell_escape(std::string_view word) return escaped_word; } -ot::status ot::run_editor(const char* editor, const char* path) +ot::status ot::run_editor(std::string_view editor, std::string_view path) { - pid_t pid = fork(); - if (pid == -1) { - return {st::standard_error, "Could not fork: "s + strerror(errno)}; - } else if (pid == 0) { - 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 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); + std::string command = std::string(editor) + " " + shell_escape(path); + int status = system(command.c_str()); - 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 %s failed: %s\n", argv[0], strerror(errno)); - wordfree(&p); - exit(1); - } - - int status = 0; - if (waitpid(pid, &status, 0) == -1) + if (status == -1) return {st::standard_error, "waitpid error: "s + strerror(errno)}; else if (!WIFEXITED(status)) return {st::child_process_failed, diff --git a/t/opustags.t b/t/opustags.t index b34a3ae..df31bf9 100755 --- a/t/opustags.t +++ b/t/opustags.t @@ -227,18 +227,18 @@ unlink('out2.opus'); # Interactive edition $ENV{EDITOR} = 'sed -i -e y/aeiou/AEIOU/ `sleep 0.1`'; -is_deeply(opustags(qw(gobble.opus -o screaming.opus -e)), ['', '', 0], 'edit a file with EDITOR'); -is(md5('screaming.opus'), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were modified'); +is_deeply(opustags('gobble.opus', '-eo', "'screaming !'.opus"), ['', '', 0], 'edit a file with EDITOR'); +is(md5("'screaming !'.opus"), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were modified'); $ENV{EDITOR} = 'true'; -is_deeply(opustags(qw(-i screaming.opus -e)), ['', "Cancelling edition because the tags file was not modified.\n", 256], 'close -e without saving'); -is(md5('screaming.opus'), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were not modified'); +is_deeply(opustags('-ie', "'screaming !'.opus"), ['', "Cancelling edition because the tags file was not modified.\n", 256], 'close -e without saving'); +is(md5("'screaming !'.opus"), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were not modified'); $ENV{EDITOR} = 'false'; -is_deeply(opustags(qw(-i screaming.opus -e)), ['', "screaming.opus: error: Child process exited with 1\n", 256], 'editor exiting with an error'); -is(md5('screaming.opus'), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were not modified'); +is_deeply(opustags('-ie', "'screaming !'.opus"), ['', "'screaming !'.opus: error: Child process exited with 1\n", 256], 'editor exiting with an error'); +is(md5("'screaming !'.opus"), '56e85ccaa83a13c15576d75bbd6d835f', 'the tags were not modified'); -unlink('screaming.opus'); +unlink("'screaming !'.opus"); #################################################################################################### # Test muxed streams