mirror/opustags
1
0
Fork 0
mirror of https://github.com/fmang/opustags.git synced 2025-01-15 12:43:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

run_editor: Pass the editor command through the shell

Browse Source 
wordexp doesn’t work on OpenBSD, and escaping the path ourselves then
calling system() is actually easier than using wordexp.
This commit is contained in:
Frédéric Mangano 2020-11-01 10:54:23 +01:00
parent 639d46ed0f
commit 7c8396ca45
5 changed files with 16 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
opustags.1
View File

@ -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:

2
src/cli.cc
View File

@ -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);

6
src/opustags.h
View File

@ -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

34
src/system.cc
View File

@ -12,11 +12,11 @@
#include <opustags.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <unistd.h>
#include <wordexp.h>
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<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);
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,

14
t/opustags.t
View File

@ -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