From 716c785e34f9b016ddf200048e7642f6eb1ffab2 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Mon, 19 Oct 2015 20:00:36 +0300 Subject: [PATCH 01/12] Alter spawn to return the error message only in error->message That is, do not cite the original text, program name, or the failed OS function name (except when the testing program is compiled). Also cut glib citing of the original text on bad quoting. --- src/spawn.c | 54 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/src/spawn.c b/src/spawn.c index bff20f8a..2e4c1971 100644 --- a/src/spawn.c +++ b/src/spawn.c @@ -74,6 +74,23 @@ # define DEFAULT_IO_LENGTH 2048 #else # define DEFAULT_IO_LENGTH 4096 + +/* helper function that cuts glib citing of the original text on bad quoting: it may be long, + and only the caller knows whether it's UTF-8. Thought we lose the ' or " failed info. */ +static gboolean spawn_parse_argv(const gchar *command_line, gint *argcp, gchar ***argvp, + GError **error) +{ + GError *gerror = NULL; + + if (g_shell_parse_argv(command_line, argcp, argvp, &gerror)) + return TRUE; + + g_set_error_literal(error, gerror->domain, gerror->code, + gerror->code == G_SHELL_ERROR_BAD_QUOTING ? + _("Text ended before matching quote was found") : gerror->message); + g_error_free(gerror); + return FALSE; +} #endif #define G_IO_FAILURE (G_IO_ERR | G_IO_HUP | G_IO_NVAL) /* always used together */ @@ -104,7 +121,7 @@ static gchar *spawn_get_program_name(const gchar *command_line, GError **error) if (!*command_line) { - g_set_error(error, G_SHELL_ERROR, G_SHELL_ERROR_EMPTY_STRING, + g_set_error_literal(error, G_SHELL_ERROR, G_SHELL_ERROR_EMPTY_STRING, /* TL note: from glib */ _("Text was empty (or contained only whitespace)")); return FALSE; @@ -119,16 +136,14 @@ static gchar *spawn_get_program_name(const gchar *command_line, GError **error) /* Windows allows "foo.exe, but we may have extra arguments */ if ((s = strchr(command_line, '"')) == NULL) { - g_set_error(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, - /* TL note: from glib */ - _("Text ended before matching quote was found for %c." - " (The text was '%s')"), '"', command_line); + g_set_error_literal(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, + _("Text ended before matching quote was found")); return FALSE; } if (!strchr(" \t", s[1])) /* strchr() catches s[1] == '\0' */ { - g_set_error(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, + g_set_error_literal(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, _("A quoted Windows program name must be entirely inside the quotes")); return FALSE; } @@ -142,7 +157,7 @@ static gchar *spawn_get_program_name(const gchar *command_line, GError **error) if (quote && quote < s) { - g_set_error(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, + g_set_error_literal(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, _("A quoted Windows program name must be entirely inside the quotes")); return FALSE; } @@ -165,10 +180,8 @@ static gchar *spawn_get_program_name(const gchar *command_line, GError **error) if (open_quote) { - g_set_error(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, - /* TL note: from glib */ - _("Text ended before matching quote was found for %c." - " (The text was '%s')"), '"', command_line); + g_set_error_literal(error, G_SHELL_ERROR, G_SHELL_ERROR_BAD_QUOTING, + _("Text ended before matching quote was found")); g_free(program); return FALSE; } @@ -176,7 +189,7 @@ static gchar *spawn_get_program_name(const gchar *command_line, GError **error) int argc; char **argv; - if (!g_shell_parse_argv(command_line, &argc, &argv, error)) + if (!spawn_parse_argv(command_line, &argc, &argv, error)) return FALSE; /* empty string results in parse error, so argv[0] is not NULL */ @@ -237,8 +250,8 @@ gboolean spawn_check_command(const gchar *command_line, gboolean execute, GError if (!executable) { - g_set_error(error, G_SHELL_ERROR, G_SHELL_ERROR_FAILED, /* or SPAWN error? */ - _("Program '%s' not found"), program); + g_set_error_literal(error, G_SHELL_ERROR, G_SHELL_ERROR_FAILED, + _("Program not found")); g_free(program); return FALSE; } @@ -274,15 +287,14 @@ gboolean spawn_kill_process(GPid pid, GError **error) { gchar *message = g_win32_error_message(GetLastError()); - g_set_error(error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, - _("TerminateProcess() failed: %s"), message); + g_set_error_literal(error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, message); g_free(message); return FALSE; } #else if (kill(pid, SIGTERM)) { - g_set_error(error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, "%s", g_strerror(errno)); + g_set_error_literal(error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, g_strerror(errno)); return FALSE; } #endif @@ -379,8 +391,12 @@ leave: if (!message) message = g_win32_error_message(GetLastError()); + #ifdef SPAWN_TEST failure = g_strdup_printf("%s() failed: %s", failed, message); g_free(message); + #else + failure = message; + #endif } if (pipe_io) @@ -546,7 +562,7 @@ static gboolean spawn_async_with_pipes(const gchar *working_directory, const gch if (failure) { - g_set_error(error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, "%s", failure); + g_set_error_literal(error, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, failure); g_free(failure); return FALSE; } @@ -562,7 +578,7 @@ static gboolean spawn_async_with_pipes(const gchar *working_directory, const gch int argc = 0; char **cl_argv; - if (!g_shell_parse_argv(command_line, &cl_argc, &cl_argv, error)) + if (!spawn_parse_argv(command_line, &cl_argc, &cl_argv, error)) return FALSE; if (argv) From d60a4b789d28dabc97801d8579e74cd1fdfee524 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Mon, 19 Oct 2015 20:43:23 +0300 Subject: [PATCH 02/12] Alter spawn to cut glib citing of the program name or working dir Mostly translate the G_SPAWN error codes back to errno, and get the curresponding g_strerror() message. --- src/spawn.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/src/spawn.c b/src/spawn.c index 2e4c1971..370e3de9 100644 --- a/src/spawn.c +++ b/src/spawn.c @@ -572,6 +572,7 @@ static gboolean spawn_async_with_pipes(const gchar *working_directory, const gch int cl_argc; char **full_argv; gboolean spawned; + GError *gerror = NULL; if (command_line) { @@ -593,7 +594,83 @@ static gboolean spawn_async_with_pipes(const gchar *working_directory, const gch spawned = g_spawn_async_with_pipes(working_directory, full_argv, envp, G_SPAWN_SEARCH_PATH | (child_pid ? G_SPAWN_DO_NOT_REAP_CHILD : 0), NULL, NULL, - child_pid, stdin_fd, stdout_fd, stderr_fd, error); + child_pid, stdin_fd, stdout_fd, stderr_fd, &gerror); + + if (!spawned) + { + gint en = 0; + const gchar *message = gerror->message; + + /* try to cut glib citing of the program name or working directory: they may be long, + and only the caller knows whether they're UTF-8. We lose the exact chdir error. */ + switch (gerror->code) + { + #ifdef EACCES + case G_SPAWN_ERROR_ACCES : en = EACCES; break; + #endif + #ifdef EPERM + case G_SPAWN_ERROR_PERM : en = EPERM; break; + #endif + #ifdef E2BIG + case G_SPAWN_ERROR_TOO_BIG : en = E2BIG; break; + #endif + #ifdef ENOEXEC + case G_SPAWN_ERROR_NOEXEC : en = ENOEXEC; break; + #endif + #ifdef ENAMETOOLONG + case G_SPAWN_ERROR_NAMETOOLONG : en = ENAMETOOLONG; break; + #endif + #ifdef ENOENT + case G_SPAWN_ERROR_NOENT : en = ENOENT; break; + #endif + #ifdef ENOMEM + case G_SPAWN_ERROR_NOMEM : en = ENOMEM; break; + #endif + #ifdef ENOTDIR + case G_SPAWN_ERROR_NOTDIR : en = ENOTDIR; break; + #endif + #ifdef ELOOP + case G_SPAWN_ERROR_LOOP : en = ELOOP; break; + #endif + #ifdef ETXTBUSY + case G_SPAWN_ERROR_TXTBUSY : en = ETXTBUSY; break; + #endif + #ifdef EIO + case G_SPAWN_ERROR_IO : en = EIO; break; + #endif + #ifdef ENFILE + case G_SPAWN_ERROR_NFILE : en = ENFILE; break; + #endif + #ifdef EMFILE + case G_SPAWN_ERROR_MFILE : en = EMFILE; break; + #endif + #ifdef EINVAL + case G_SPAWN_ERROR_INVAL : en = EINVAL; break; + #endif + #ifdef EISDIR + case G_SPAWN_ERROR_ISDIR : en = EISDIR; break; + #endif + #ifdef ELIBBAD + case G_SPAWN_ERROR_LIBBAD : en = ELIBBAD; break; + #endif + case G_SPAWN_ERROR_CHDIR : + { + message = _("Failed to change to the working directory"); + break; + } + case G_SPAWN_ERROR_FAILED : + { + message = _("Unknown error executing child process"); + break; + } + } + + if (en) + message = g_strerror(en); + + g_set_error_literal(error, gerror->domain, gerror->code, message); + g_error_free(gerror); + } if (full_argv != argv) { From 5aa0b02f00e84c6412be8632432726c5894e1ff3 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:22:51 +0300 Subject: [PATCH 03/12] Sync spawn glib/unix and Windows messages In particular, the exec() and CreateProcess() errors are reported directly, but failures in other any functions, which are extremely rare, include some descriptive text, such as "Failed to set pipe handle to inheritable (Access denied)". The example is artificial. --- src/spawn.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/spawn.c b/src/spawn.c index 370e3de9..e72f2fb9 100644 --- a/src/spawn.c +++ b/src/spawn.c @@ -334,7 +334,7 @@ static gchar *spawn_create_process_with_pipes(char *command_line, const char *wo if (!CreatePipe(&hpipe[pindex[i][0]], &hpipe[pindex[i][1]], NULL, 0)) { hpipe[pindex[i][0]] = hpipe[pindex[i][1]] = NULL; - failed = "CreatePipe"; + failed = "create pipe"; goto leave; } @@ -344,21 +344,21 @@ static gchar *spawn_create_process_with_pipes(char *command_line, const char *wo if ((*fd[i] = _open_osfhandle((intptr_t) hpipe[i], mode[i])) == -1) { - failed = "_open_osfhandle"; + failed = "convert pipe handle to file descriptor"; message = g_strdup(g_strerror(errno)); goto leave; } } else if (!CloseHandle(hpipe[i])) { - failed = "CloseHandle"; + failed = "close pipe"; goto leave; } if (!SetHandleInformation(hpipe[i + 3], HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)) { - failed = "SetHandleInformation"; + failed = "set pipe handle to inheritable"; goto leave; } } @@ -371,7 +371,7 @@ static gchar *spawn_create_process_with_pipes(char *command_line, const char *wo if (!CreateProcess(NULL, command_line, NULL, NULL, TRUE, pipe_io ? CREATE_NO_WINDOW : 0, environment, working_directory, &startup, &process)) { - failed = "CreateProcess"; + failed = ""; /* report the message only */ /* further errors will not be reported */ } else @@ -389,14 +389,24 @@ leave: if (failed) { if (!message) - message = g_win32_error_message(GetLastError()); + { + size_t len; - #ifdef SPAWN_TEST - failure = g_strdup_printf("%s() failed: %s", failed, message); - g_free(message); - #else - failure = message; - #endif + message = g_win32_error_message(GetLastError()); + len = strlen(message); + + /* unlike g_strerror(), the g_win32_error messages may include a final '.' */ + if (len > 0 && message[len - 1] == '.') + message[len - 1] = '\0'; + } + + if (*failed == '\0') + failure = message; + else + { + failure = g_strdup_printf("Failed to %s (%s)", failed, message); + g_free(message); + } } if (pipe_io) From e74dc40e1134e3bb7df9e45bc95f9e3c71507841 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:30:00 +0300 Subject: [PATCH 04/12] Unify the spawn callers error messages: grep tool --- src/search.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/search.c b/src/search.c index 7ae34ee7..17aa6ad1 100644 --- a/src/search.c +++ b/src/search.c @@ -1715,8 +1715,8 @@ search_find_in_files(const gchar *utf8_search_text, const gchar *utf8_dir, const } else { - geany_debug("%s: spawn_with_callbacks() failed: %s", G_STRFUNC, error->message); - ui_set_statusbar(TRUE, _("Process failed (%s)"), error->message); + ui_set_statusbar(TRUE, _("Cannot execute grep tool \"%s\": %s. " + "Check the path setting in Preferences."), tool_prefs.grep_cmd, error->message); g_error_free(error); } From 421f8a0ce08178bc5d76fd7922c253994492bf1d Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:31:37 +0300 Subject: [PATCH 05/12] Unify the spawn callers error messages: terminal command --- src/build.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/build.c b/src/build.c index 6703b16c..09a218e8 100644 --- a/src/build.c +++ b/src/build.c @@ -909,8 +909,8 @@ static void build_run_cmd(GeanyDocument *doc, guint cmdindex) } else { - geany_debug("spawn_async() failed: %s", error->message); - ui_set_statusbar(TRUE, _("Process failed (%s)"), error->message); + ui_set_statusbar(TRUE, _("Cannot execute terminal command \"%s\": %s. " + "Check the path setting in Preferences."), tool_prefs.term_cmd, error->message); g_error_free(error); g_unlink(run_cmd); run_info[cmdindex].pid = (GPid) 0; From 897197ae51cb51c7f07281ef7c6e118b27f25718 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:33:41 +0300 Subject: [PATCH 06/12] Unify the spawn callers error messages: context action --- src/callbacks.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/callbacks.c b/src/callbacks.c index e758533b..aa3be0cf 100644 --- a/src/callbacks.c +++ b/src/callbacks.c @@ -1452,6 +1452,7 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data) gchar *word, *command; GError *error = NULL; GeanyDocument *doc = document_get_current(); + const gchar *format; g_return_if_fail(doc != NULL); @@ -1469,22 +1470,29 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data) !EMPTY(doc->file_type->context_action_cmd)) { command = g_strdup(doc->file_type->context_action_cmd); + format = _("Cannot execute context action command \"%s\": %s. " + "Check the path setting in Preferences."); } else { command = g_strdup(tool_prefs.context_action_cmd); + format = _("Cannot execute context action command \"%s\": %s. " + "Check the path setting in Filetype configuration."); } /* substitute the wildcard %s and run the command if it is non empty */ if (G_LIKELY(!EMPTY(command))) { - utils_str_replace_all(&command, "%s", word); + gchar *command_line = g_strdup(command); - if (!spawn_async(NULL, command, NULL, NULL, NULL, &error)) + utils_str_replace_all(&command_line, "%s", word); + + if (!spawn_async(NULL, command_line, NULL, NULL, NULL, &error)) { - ui_set_statusbar(TRUE, "Context action command failed: %s", error->message); + ui_set_statusbar(TRUE, format, command, error->message); g_error_free(error); } + g_free(command_line); } g_free(word); g_free(command); From 389c0a223dc391a1a2a84891ab2411e76a92d64b Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:35:12 +0300 Subject: [PATCH 07/12] Unify the spawn callers error messages: print command --- src/printing.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/printing.c b/src/printing.c index 79bb0eef..535dc133 100644 --- a/src/printing.c +++ b/src/printing.c @@ -612,8 +612,9 @@ static void print_external(GeanyDocument *doc) #endif { dialogs_show_msgbox(GTK_MESSAGE_ERROR, - _("Printing of \"%s\" failed (return code: %s)."), - doc->file_name, error->message); + _("Cannot execute print command \"%s\": %s. " + "Check the path setting in Preferences."), + printing_prefs.external_print_cmd, error->message); g_error_free(error); } else From d8318b6366369b6c94eb2e709c3e5d0d9b4efd3d Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:37:23 +0300 Subject: [PATCH 08/12] Unify the spawn callers error messages: template replacement command --- src/templates.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/templates.c b/src/templates.c index fe8fee23..4f01ec63 100644 --- a/src/templates.c +++ b/src/templates.c @@ -617,7 +617,8 @@ static gchar *run_command(const gchar *command, const gchar *file_name, } else { - g_warning("templates_replace_command: %s", error->message); + g_warning(_("Cannot execute template replacement command \"%s\": %s. " + "Check the path setting in template."), command, error->message); g_error_free(error); } From 748f5689ccf58b5172d3ec80b67bbde4cc3ea1a5 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Tue, 20 Oct 2015 20:38:37 +0300 Subject: [PATCH 09/12] Unify the spawn callers error messages: custom command --- src/tools.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tools.c b/src/tools.c index c8ebc978..72128197 100644 --- a/src/tools.c +++ b/src/tools.c @@ -239,8 +239,9 @@ void tools_execute_custom_command(GeanyDocument *doc, const gchar *command) } else { - geany_debug("spawn_sync() failed: %s", error->message); - ui_set_statusbar(TRUE, _("Custom command failed: %s"), error->message); + ui_set_statusbar(TRUE, _("Cannot execute custom command \"%s\": %s. " + "Check the path setting in Preferences."), command, error->message); + g_error_free(error); g_error_free(error); } From ca76d87cac33d59c0b726a4f47d47b7948aca55e Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Sun, 1 Nov 2015 15:34:26 +0200 Subject: [PATCH 10/12] Fix context action spawning error message Properly display what configuration should be checked, and show the entire command line on parsing error. --- src/callbacks.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/callbacks.c b/src/callbacks.c index aa3be0cf..11c4083b 100644 --- a/src/callbacks.c +++ b/src/callbacks.c @@ -1452,7 +1452,7 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data) gchar *word, *command; GError *error = NULL; GeanyDocument *doc = document_get_current(); - const gchar *format; + const gchar *check_msg; g_return_if_fail(doc != NULL); @@ -1470,14 +1470,12 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data) !EMPTY(doc->file_type->context_action_cmd)) { command = g_strdup(doc->file_type->context_action_cmd); - format = _("Cannot execute context action command \"%s\": %s. " - "Check the path setting in Preferences."); + check_msg = _("Check the path setting in Filetype configuration."); } else { command = g_strdup(tool_prefs.context_action_cmd); - format = _("Cannot execute context action command \"%s\": %s. " - "Check the path setting in Filetype configuration."); + check_msg = _("Check the path setting in Preferences."); } /* substitute the wildcard %s and run the command if it is non empty */ @@ -1489,7 +1487,10 @@ void on_context_action1_activate(GtkMenuItem *menuitem, gpointer user_data) if (!spawn_async(NULL, command_line, NULL, NULL, NULL, &error)) { - ui_set_statusbar(TRUE, format, command, error->message); + /* G_SHELL_ERROR is parsing error, it may be caused by %s word with quotes */ + ui_set_statusbar(TRUE, _("Cannot execute context action command \"%s\": %s. %s"), + error->domain == G_SHELL_ERROR ? command_line : command, error->message, + check_msg); g_error_free(error); } g_free(command_line); From a99454f4a0b740085bd5a5974aab8e5b12615db3 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Sun, 1 Nov 2015 16:00:03 +0200 Subject: [PATCH 11/12] Clarify the template command spawning message --- src/templates.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/templates.c b/src/templates.c index 4f01ec63..512edb04 100644 --- a/src/templates.c +++ b/src/templates.c @@ -617,8 +617,8 @@ static gchar *run_command(const gchar *command, const gchar *file_name, } else { - g_warning(_("Cannot execute template replacement command \"%s\": %s. " - "Check the path setting in template."), command, error->message); + g_warning(_("Cannot execute command \"%s\" from the template: %s. " + "Check the path in the template."), command, error->message); g_error_free(error); } From 63908309533731ecdd8865e4cf118db1005e1bb4 Mon Sep 17 00:00:00 2001 From: Dimitar Zhekov Date: Sun, 1 Nov 2015 16:19:40 +0200 Subject: [PATCH 12/12] Fix custom command spawning error message They are configured in Custom Commands, not Preferences. Also fix the duplicated g_error_free(). --- src/tools.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools.c b/src/tools.c index 72128197..0276aec9 100644 --- a/src/tools.c +++ b/src/tools.c @@ -240,8 +240,7 @@ void tools_execute_custom_command(GeanyDocument *doc, const gchar *command) else { ui_set_statusbar(TRUE, _("Cannot execute custom command \"%s\": %s. " - "Check the path setting in Preferences."), command, error->message); - g_error_free(error); + "Check the path setting in Custom Commands."), command, error->message); g_error_free(error); }