From a7f7d4a488c8d3b2461122765f9904c8a411a583 Mon Sep 17 00:00:00 2001 From: Brian Ashworth Date: Thu, 2 Aug 2018 21:37:29 -0400 Subject: [PATCH] Write to swaynag pipe fd directly on config errors --- include/sway/commands.h | 1 + include/sway/config.h | 14 +++-- include/sway/swaynag.h | 33 ++++++++++ sway/commands.c | 8 +-- sway/commands/include.c | 15 +---- sway/commands/reload.c | 15 +---- sway/commands/swaynag_command.c | 20 ++++++ sway/config.c | 104 ++++++++++++-------------------- sway/main.c | 15 ++--- sway/meson.build | 2 + sway/sway.5.scd | 7 +++ sway/swaynag.c | 103 +++++++++++++++++++++++++++++++ 12 files changed, 227 insertions(+), 110 deletions(-) create mode 100644 include/sway/swaynag.h create mode 100644 sway/commands/swaynag_command.c create mode 100644 sway/swaynag.c diff --git a/include/sway/commands.h b/include/sway/commands.h index 41858ccca..f83907b2c 100644 --- a/include/sway/commands.h +++ b/include/sway/commands.h @@ -150,6 +150,7 @@ sway_cmd cmd_splitt; sway_cmd cmd_splitv; sway_cmd cmd_sticky; sway_cmd cmd_swaybg_command; +sway_cmd cmd_swaynag_command; sway_cmd cmd_swap; sway_cmd cmd_title_format; sway_cmd cmd_unmark; diff --git a/include/sway/config.h b/include/sway/config.h index 4fc3eadb2..632aca143 100644 --- a/include/sway/config.h +++ b/include/sway/config.h @@ -7,6 +7,7 @@ #include #include #include "list.h" +#include "swaynag.h" #include "tree/layout.h" #include "tree/container.h" #include "wlr-layer-shell-unstable-v1-protocol.h" @@ -308,7 +309,8 @@ enum focus_wrapping_mode { * The configuration struct. The result of loading a config file. */ struct sway_config { - pid_t swaynag_pid; + char *swaynag_command; + struct swaynag_instance swaynag_config_errors; list_t *symbols; list_t *modes; list_t *bars; @@ -346,6 +348,7 @@ struct sway_config { bool failed; bool reloading; bool reading; + bool validating; bool auto_back_and_forth; bool show_marks; @@ -404,18 +407,19 @@ struct sway_config { * Loads the main config from the given path. is_active should be true when * reloading the config. */ -bool load_main_config(const char *path, bool is_active, char **errors); +bool load_main_config(const char *path, bool is_active, bool validating); /** * Loads an included config. Can only be used after load_main_config. */ bool load_include_configs(const char *path, struct sway_config *config, - char **errors); + struct swaynag_instance *swaynag); /** * Reads the config from the given FILE. */ -bool read_config(FILE *file, struct sway_config *config, char **errors); +bool read_config(FILE *file, struct sway_config *config, + struct swaynag_instance *swaynag); /** * Free config struct @@ -424,8 +428,6 @@ void free_config(struct sway_config *config); void free_sway_variable(struct sway_variable *var); -void spawn_swaynag_config_errors(struct sway_config *config, char *errors); - /** * Does variable replacement for a string based on the config's currently loaded variables. */ diff --git a/include/sway/swaynag.h b/include/sway/swaynag.h new file mode 100644 index 000000000..ac0086a16 --- /dev/null +++ b/include/sway/swaynag.h @@ -0,0 +1,33 @@ +#ifndef _SWAY_SWAYNAG_H +#define _SWAY_SWAYNAG_H + +struct swaynag_instance { + const char *args; + pid_t pid; + int fd[2]; + bool detailed; +}; + +// Copy all fields of one instance to another +void swaynag_clone(struct swaynag_instance *dest, + struct swaynag_instance *src); + +// Spawn swaynag. If swaynag->detailed, then swaynag->fd[1] will left open +// so it can be written to. Call swaynag_show when done writing. This will +// be automatically called by swaynag_log if the instance is not spawned and +// swaynag->detailed is true. +bool swaynag_spawn(const char *swaynag_command, + struct swaynag_instance *swaynag); + +// Kill the swaynag instance +void swaynag_kill(struct swaynag_instance *swaynag); + +// Write a log message to swaynag->fd[1]. This will fail when swaynag->detailed +// is false. +void swaynag_log(const char *swaynag_command, struct swaynag_instance *swaynag, + const char *fmt, ...); + +// If swaynag->detailed, close swaynag->fd[1] so swaynag displays +void swaynag_show(struct swaynag_instance *swaynag); + +#endif diff --git a/sway/commands.c b/sway/commands.c index 81e9ea420..364c26da8 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -132,6 +132,7 @@ static struct cmd_handler handlers[] = { static struct cmd_handler config_handlers[] = { { "default_orientation", cmd_default_orientation }, { "swaybg_command", cmd_swaybg_command }, + { "swaynag_command", cmd_swaynag_command }, { "workspace_layout", cmd_workspace_layout }, }; @@ -511,14 +512,11 @@ struct cmd_results *cmd_results_new(enum cmd_status status, results->input = NULL; } if (format) { + char *error = malloc(256); va_list args; va_start(args, format); - size_t length = vsnprintf(NULL, 0, format, args) + 1; - char *error = malloc(length); - va_end(args); - va_start(args, format); if (error) { - vsnprintf(error, length, format, args); + vsnprintf(error, 256, format, args); } va_end(args); results->error = error; diff --git a/sway/commands/include.c b/sway/commands/include.c index 72fec7cce..61f383bb0 100644 --- a/sway/commands/include.c +++ b/sway/commands/include.c @@ -7,19 +7,10 @@ struct cmd_results *cmd_include(int argc, char **argv) { return error; } - char *errors = NULL; - if (!load_include_configs(argv[0], config, &errors)) { - struct cmd_results *result = cmd_results_new(CMD_INVALID, "include", + if (!load_include_configs(argv[0], config, + &config->swaynag_config_errors)) { + return cmd_results_new(CMD_INVALID, "include", "Failed to include sub configuration file: %s", argv[0]); - free(errors); - return result; - } - - if (errors) { - struct cmd_results *result = cmd_results_new(CMD_INVALID, "include", - "There are errors in the included config\n%s", errors); - free(errors); - return result; } return cmd_results_new(CMD_SUCCESS, NULL, NULL); diff --git a/sway/commands/reload.c b/sway/commands/reload.c index 9bf671d94..f8ca374d0 100644 --- a/sway/commands/reload.c +++ b/sway/commands/reload.c @@ -1,5 +1,4 @@ #define _XOPEN_SOURCE 500 -#include #include #include "sway/commands.h" #include "sway/config.h" @@ -20,9 +19,7 @@ struct cmd_results *cmd_reload(int argc, char **argv) { list_add(bar_ids, strdup(bar->id)); } - char *errors = NULL; - if (!load_main_config(config->current_config_path, true, &errors)) { - free(errors); + if (!load_main_config(config->current_config_path, true, false)) { return cmd_results_new(CMD_FAILURE, "reload", "Error(s) reloading config."); } @@ -47,15 +44,5 @@ struct cmd_results *cmd_reload(int argc, char **argv) { arrange_windows(&root_container); - if (config->swaynag_pid > 0) { - kill(config->swaynag_pid, SIGTERM); - config->swaynag_pid = -1; - } - - if (errors) { - spawn_swaynag_config_errors(config, errors); - free(errors); - } - return cmd_results_new(CMD_SUCCESS, NULL, NULL); } diff --git a/sway/commands/swaynag_command.c b/sway/commands/swaynag_command.c new file mode 100644 index 000000000..c57a80a64 --- /dev/null +++ b/sway/commands/swaynag_command.c @@ -0,0 +1,20 @@ +#include +#include "sway/commands.h" +#include "log.h" +#include "stringop.h" + +struct cmd_results *cmd_swaynag_command(int argc, char **argv) { + struct cmd_results *error = NULL; + if ((error = checkarg(argc, "swaynag_command", EXPECTED_AT_LEAST, 1))) { + return error; + } + + if (config->swaynag_command) { + free(config->swaynag_command); + } + config->swaynag_command = join_args(argv, argc); + wlr_log(WLR_DEBUG, "Using custom swaynag command: %s", + config->swaynag_command); + + return cmd_results_new(CMD_SUCCESS, NULL, NULL); +} diff --git a/sway/config.c b/sway/config.c index bd282541c..4464b0068 100644 --- a/sway/config.c +++ b/sway/config.c @@ -25,6 +25,7 @@ #include "sway/commands.h" #include "sway/config.h" #include "sway/criteria.h" +#include "sway/swaynag.h" #include "sway/tree/arrange.h" #include "sway/tree/layout.h" #include "sway/tree/workspace.h" @@ -72,6 +73,8 @@ void free_config(struct sway_config *config) { memset(&config->handler_context, 0, sizeof(config->handler_context)); + free(config->swaynag_command); + // TODO: handle all currently unhandled lists as we add implementations if (config->symbols) { for (int i = 0; i < config->symbols->length; ++i) { @@ -158,7 +161,17 @@ static void set_color(float dest[static 4], uint32_t color) { } static void config_defaults(struct sway_config *config) { - config->swaynag_pid = -1; + config->swaynag_command = strdup("swaynag"); + config->swaynag_config_errors = (struct swaynag_instance){ + .args = "--type error " + "--message 'There are errors in your config file' " + "--detailed-message " + "--button 'Exit sway' 'swaymsg exit' " + "--button 'Reload sway' 'swaymsg reload'", + .pid = -1, + .detailed = true, + }; + if (!(config->symbols = create_list())) goto cleanup; if (!(config->modes = create_list())) goto cleanup; if (!(config->bars = create_list())) goto cleanup; @@ -205,6 +218,7 @@ static void config_defaults(struct sway_config *config) { config->focus_follows_mouse = true; config->mouse_warping = true; config->focus_wrapping = WRAP_YES; + config->validating = false; config->reloading = false; config->active = false; config->failed = false; @@ -321,7 +335,7 @@ static char *get_config_path(void) { } static bool load_config(const char *path, struct sway_config *config, - char **errors) { + struct swaynag_instance *swaynag) { if (path == NULL) { wlr_log(WLR_ERROR, "Unable to find a config file!"); return false; @@ -340,7 +354,7 @@ static bool load_config(const char *path, struct sway_config *config, return false; } - bool config_load_success = read_config(f, config, errors); + bool config_load_success = read_config(f, config, swaynag); fclose(f); if (!config_load_success) { @@ -350,7 +364,7 @@ static bool load_config(const char *path, struct sway_config *config, return true; } -bool load_main_config(const char *file, bool is_active, char **errors) { +bool load_main_config(const char *file, bool is_active, bool validating) { char *path; if (file != NULL) { path = strdup(file); @@ -365,11 +379,16 @@ bool load_main_config(const char *file, bool is_active, char **errors) { } config_defaults(config); + config->validating = validating; if (is_active) { wlr_log(WLR_DEBUG, "Performing configuration file reload"); - config->swaynag_pid = old_config->swaynag_pid; config->reloading = true; config->active = true; + + swaynag_kill(&old_config->swaynag_config_errors); + swaynag_clone(&config->swaynag_config_errors, + &old_config->swaynag_config_errors); + create_default_output_configs(); } @@ -426,13 +445,17 @@ bool load_main_config(const char *file, bool is_active, char **errors) { } */ - success = success && load_config(path, config, errors); + success = success && load_config(path, config, + &config->swaynag_config_errors); if (is_active) { for (int i = 0; i < config->output_configs->length; i++) { apply_output_config_to_outputs(config->output_configs->items[i]); } config->reloading = false; + if (config->swaynag_config_errors.pid > 0) { + swaynag_show(&config->swaynag_config_errors); + } } if (old_config) { @@ -444,7 +467,7 @@ bool load_main_config(const char *file, bool is_active, char **errors) { } static bool load_include_config(const char *path, const char *parent_dir, - struct sway_config *config, char **errors) { + struct sway_config *config, struct swaynag_instance *swaynag) { // save parent config const char *parent_config = config->current_config_path; @@ -488,7 +511,7 @@ static bool load_include_config(const char *path, const char *parent_dir, list_add(config->config_chain, real_path); int index = config->config_chain->length - 1; - if (!load_config(real_path, config, errors)) { + if (!load_config(real_path, config, swaynag)) { free(real_path); config->current_config_path = parent_config; list_del(config->config_chain, index); @@ -501,7 +524,7 @@ static bool load_include_config(const char *path, const char *parent_dir, } bool load_include_configs(const char *path, struct sway_config *config, - char **errors) { + struct swaynag_instance *swaynag) { char *wd = getcwd(NULL, 0); char *parent_path = strdup(config->current_config_path); const char *parent_dir = dirname(parent_path); @@ -523,7 +546,7 @@ bool load_include_configs(const char *path, struct sway_config *config, char **w = p.we_wordv; size_t i; for (i = 0; i < p.we_wordc; ++i) { - load_include_config(w[i], parent_dir, config, errors); + load_include_config(w[i], parent_dir, config, swaynag); } free(parent_path); wordfree(&p); @@ -579,26 +602,8 @@ static char *expand_line(const char *block, const char *line, bool add_brace) { return expanded; } -static void log_error(char **errors, const char *fmt, ...) { - va_list args; - va_start(args, fmt); - size_t length = vsnprintf(NULL, 0, fmt, args) + 1; - va_end(args); - - int offset = *errors ? strlen(*errors) : 0; - char *temp = realloc(*errors, offset + length + 1); - if (!temp) { - wlr_log(WLR_ERROR, "Failed to realloc error log"); - return; - } - *errors = temp; - - va_start(args, fmt); - vsnprintf(*errors + offset, length, fmt, args); - va_end(args); -} - -bool read_config(FILE *file, struct sway_config *config, char **errors) { +bool read_config(FILE *file, struct sway_config *config, + struct swaynag_instance *swaynag) { bool reading_main_config = false; char *this_config = NULL; size_t config_size = 0; @@ -688,8 +693,11 @@ bool read_config(FILE *file, struct sway_config *config, char **errors) { case CMD_INVALID: wlr_log(WLR_ERROR, "Error on line %i '%s': %s (%s)", line_number, line, res->error, config->current_config_path); - log_error(errors, "Error on line %i (%s) '%s': %s\n", line_number, - config->current_config_path, line, res->error); + if (!config->validating) { + swaynag_log(config->swaynag_command, swaynag, + "Error on line %i (%s) '%s': %s", line_number, + config->current_config_path, line, res->error); + } success = false; break; @@ -738,38 +746,6 @@ bool read_config(FILE *file, struct sway_config *config, char **errors) { return success; } -void spawn_swaynag_config_errors(struct sway_config *config, char *errors) { - char *command = "swaynag " - "--type error " - "--message 'There are errors in your config file' " - "--detailed-message " - "--button 'Exit sway' 'swaymsg exit' " - "--button 'Reload sway' 'swaymsg reload'"; - - int fd[2]; - if (pipe(fd) != 0) { - wlr_log(WLR_ERROR, "Failed to create pipe for swaynag"); - return; - } - - pid_t pid; - if ((pid = fork()) == 0) { - close(fd[1]); - dup2(fd[0], STDIN_FILENO); - close(fd[0]); - execl("/bin/sh", "/bin/sh", "-c", command, NULL); - _exit(0); - } else if (pid < 0) { - wlr_log(WLR_ERROR, "Failed to create fork for swaynag"); - } - - close(fd[0]); - write(fd[1], errors, strlen(errors)); - close(fd[1]); - - config->swaynag_pid = pid; -} - char *do_var_replacement(char *str) { int i; char *find = str; diff --git a/sway/main.c b/sway/main.c index de2445a86..c02caf424 100644 --- a/sway/main.c +++ b/sway/main.c @@ -22,6 +22,7 @@ #include "sway/debug.h" #include "sway/desktop/transaction.h" #include "sway/server.h" +#include "sway/swaynag.h" #include "sway/tree/layout.h" #include "sway/ipc-server.h" #include "ipc-client.h" @@ -415,14 +416,13 @@ int main(int argc, char **argv) { ipc_init(&server); log_env(); - char *errors = NULL; if (validate) { - bool valid = load_main_config(config_path, false, &errors); - free(errors); + bool valid = load_main_config(config_path, false, true); return valid ? 0 : 1; } - if (!load_main_config(config_path, false, &errors)) { + setenv("WAYLAND_DISPLAY", server.socket, true); + if (!load_main_config(config_path, false, false)) { sway_terminate(EXIT_FAILURE); } @@ -432,10 +432,8 @@ int main(int argc, char **argv) { security_sanity_check(); - setenv("WAYLAND_DISPLAY", server.socket, true); if (!terminate_request) { if (!server_start_backend(&server)) { - free(errors); sway_terminate(EXIT_FAILURE); } } @@ -455,9 +453,8 @@ int main(int argc, char **argv) { } transaction_commit_dirty(); - if (errors) { - spawn_swaynag_config_errors(config, errors); - free(errors); + if (config->swaynag_config_errors.pid > 0) { + swaynag_show(&config->swaynag_config_errors); } if (!terminate_request) { diff --git a/sway/meson.build b/sway/meson.build index a9503c3b8..17406f6b9 100644 --- a/sway/meson.build +++ b/sway/meson.build @@ -9,6 +9,7 @@ sway_sources = files( 'ipc-server.c', 'scratchpad.c', 'security.c', + 'swaynag.c', 'desktop/desktop.c', 'desktop/idle_inhibit_v1.c', @@ -78,6 +79,7 @@ sway_sources = files( 'commands/split.c', 'commands/sticky.c', 'commands/swaybg_command.c', + 'commands/swaynag_command.c', 'commands/swap.c', 'commands/title_format.c', 'commands/unmark.c', diff --git a/sway/sway.5.scd b/sway/sway.5.scd index d369d7b65..82df38e3d 100644 --- a/sway/sway.5.scd +++ b/sway/sway.5.scd @@ -59,6 +59,13 @@ The following commands may only be used in the configuration file. Executes custom background _command_. Default is _swaybg_. Refer to *output* below for more information. +*swaynag\_command* + Executes custom command for _swaynag_. Default is _swaynag_. Additional + arguments may be appended to the end. This should only be used to either + direct sway to call swaynag from a custom path or to provide additional + arguments. This should be placed at the top of the config for the best + results. + The following commands cannot be used directly in the configuration file. They are expected to be used with *bindsym* or at runtime through *swaymsg*(1). diff --git a/sway/swaynag.c b/sway/swaynag.c new file mode 100644 index 000000000..2dc0cb216 --- /dev/null +++ b/sway/swaynag.c @@ -0,0 +1,103 @@ +#include +#include +#include +#include +#include +#include +#include +#include "log.h" +#include "sway/swaynag.h" + +void swaynag_clone(struct swaynag_instance *dest, + struct swaynag_instance *src) { + dest->args = src->args; + dest->pid = src->pid; + dest->fd[0] = src->fd[0]; + dest->fd[1] = src->fd[1]; + dest->detailed = src->detailed; +} + +bool swaynag_spawn(const char *swaynag_command, + struct swaynag_instance *swaynag) { + if (swaynag->detailed) { + if (pipe(swaynag->fd) != 0) { + wlr_log(WLR_ERROR, "Failed to create pipe for swaynag"); + return false; + } + fcntl(swaynag->fd[1], F_SETFD, FD_CLOEXEC); + } + + pid_t pid; + if ((pid = fork()) == 0) { + if (swaynag->detailed) { + close(swaynag->fd[1]); + dup2(swaynag->fd[0], STDIN_FILENO); + close(swaynag->fd[0]); + } + + size_t length = strlen(swaynag_command) + strlen(swaynag->args) + 2; + char *cmd = malloc(length); + snprintf(cmd, length, "%s %s", swaynag_command, swaynag->args); + execl("/bin/sh", "/bin/sh", "-c", cmd, NULL); + _exit(0); + } else if (pid < 0) { + wlr_log(WLR_ERROR, "Failed to create fork for swaynag"); + if (swaynag->detailed) { + close(swaynag->fd[0]); + close(swaynag->fd[1]); + } + return false; + } + + if (swaynag->detailed) { + close(swaynag->fd[0]); + } + swaynag->pid = pid; + return true; +} + + +void swaynag_kill(struct swaynag_instance *swaynag) { + if (swaynag->pid > 0) { + kill(swaynag->pid, SIGTERM); + swaynag->pid = -1; + } +} + +void swaynag_log(const char *swaynag_command, struct swaynag_instance *swaynag, + const char *fmt, ...) { + if (!swaynag->detailed) { + wlr_log(WLR_ERROR, "Attempting to write to non-detailed swaynag inst"); + return; + } + + if (swaynag->pid <= 0 && !swaynag_spawn(swaynag_command, swaynag)) { + return; + } + + va_list args; + va_start(args, fmt); + size_t length = vsnprintf(NULL, 0, fmt, args) + 1; + va_end(args); + + char *temp = malloc(length + 1); + if (!temp) { + wlr_log(WLR_ERROR, "Failed to allocate buffer for swaynag log entry."); + return; + } + + va_start(args, fmt); + vsnprintf(temp, length, fmt, args); + va_end(args); + + write(swaynag->fd[1], temp, length); + + free(temp); +} + +void swaynag_show(struct swaynag_instance *swaynag) { + if (swaynag->detailed && swaynag->pid > 0) { + close(swaynag->fd[1]); + } +} +