From 182e5b432893f178524935f20b4f9f6d0f0d9c80 Mon Sep 17 00:00:00 2001 From: Hofi Date: Mon, 24 Feb 2025 19:24:40 +0100 Subject: [PATCH 1/9] syslog-ng-ctl: do not override the default help detection of glib help context This way syslog-ng-ctl -h, syslog-ng-ctl--help and syslog-ng-ctl command -h, syslog-ng-ctl command --help will behave the same way (not like till now) Signed-off-by: Hofi --- syslog-ng-ctl/syslog-ng-ctl.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/syslog-ng-ctl/syslog-ng-ctl.c b/syslog-ng-ctl/syslog-ng-ctl.c index 0e2fd45aa6..94782c8ec3 100644 --- a/syslog-ng-ctl/syslog-ng-ctl.c +++ b/syslog-ng-ctl/syslog-ng-ctl.c @@ -143,12 +143,6 @@ print_usage(const gchar *bin_name, CommandDescriptor *descriptors) } } -gboolean -_is_help(gchar *cmd) -{ - return g_str_equal(cmd, "--help"); -} - static CommandDescriptor * find_active_mode(CommandDescriptor descriptors[], gint *argc, char **argv, GString *cmdname_accumulator) { @@ -197,12 +191,6 @@ main(int argc, char *argv[]) control_name = get_installation_path_for(PATH_CONTROL_SOCKET); - if (argc > 1 && _is_help(argv[1])) - { - print_usage(argv[0], modes); - exit(0); - } - GString *cmdname_accumulator = g_string_new(argv[0]); CommandDescriptor *active_mode = find_active_mode(modes, &argc, argv, cmdname_accumulator); GOptionContext *ctx = setup_help_context(cmdname_accumulator->str, active_mode); From 3271b4b0028ba4d46f86774f5ba597b133656afe Mon Sep 17 00:00:00 2001 From: Hofi Date: Mon, 24 Feb 2025 19:27:40 +0100 Subject: [PATCH 2/9] syslog-ng-ctl: consolidated and unified the help printing Signed-off-by: Hofi --- syslog-ng-ctl/syslog-ng-ctl.c | 46 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/syslog-ng-ctl/syslog-ng-ctl.c b/syslog-ng-ctl/syslog-ng-ctl.c index 94782c8ec3..85ad822c68 100644 --- a/syslog-ng-ctl/syslog-ng-ctl.c +++ b/syslog-ng-ctl/syslog-ng-ctl.c @@ -50,7 +50,7 @@ #endif static const gchar *control_name; -static void print_usage(const gchar *bin_name, CommandDescriptor *descriptors); +static void print_usage(const gchar *bin_name, const gchar *command, CommandDescriptor *descriptors); static gint slng_stop(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) @@ -132,24 +132,34 @@ static CommandDescriptor modes[] = }; static void -print_usage(const gchar *bin_name, CommandDescriptor *descriptors) +print_usage(const gchar *bin_name, const gchar *command, CommandDescriptor *descriptors) { gint mode; - - fprintf(stderr, "Syntax: %s [options]\nPossible commands are:\n", bin_name); + gboolean has_command = command && *command; + + // NOTE: this does not handle more than 2 commands, but currently we do not have such a case + fprintf(stderr, "Usage:\n %s%s%s <%s[command]> [options]\n\nPossible commands are:\n", + bin_name, + has_command ? " " : "", + has_command ? command : "", + has_command ? "" : "command "); for (mode = 0; descriptors[mode].mode; mode++) { fprintf(stderr, " %-20s %s\n", descriptors[mode].mode, descriptors[mode].description); } + fprintf(stderr, + "\nFor detailed help of a given command use:\n %s --help|-h\n %s --help|-h \n\nFor options of the given command use:\n %s [option] --help|-h\n %s --help|-h [option]\n", + bin_name, bin_name, bin_name, bin_name); } static CommandDescriptor * -find_active_mode(CommandDescriptor descriptors[], gint *argc, char **argv, GString *cmdname_accumulator) +find_active_mode(GString *bin_name, CommandDescriptor descriptors[], gint *argc, char **argv, + GString *cmdname_accumulator) { const gchar *mode_string = get_mode(argc, &argv); if (!mode_string) { - print_usage(cmdname_accumulator->str, descriptors); + print_usage(bin_name->str, cmdname_accumulator->str, descriptors); exit(1); } @@ -157,11 +167,14 @@ find_active_mode(CommandDescriptor descriptors[], gint *argc, char **argv, GStri if (strcmp(descriptors[mode].mode, mode_string) == 0) { if (descriptors[mode].main) - return &descriptors[mode]; + { + g_string_append_printf(cmdname_accumulator, "%s%s", cmdname_accumulator->len > 0 ? " " : "", mode_string); + return &descriptors[mode]; + } g_assert(descriptors[mode].subcommands); - g_string_append_printf(cmdname_accumulator, " %s", mode_string); - return find_active_mode(descriptors[mode].subcommands, argc, argv, cmdname_accumulator); + g_string_append_printf(cmdname_accumulator, "%s%s", cmdname_accumulator->len > 0 ? " " : "", mode_string); + return find_active_mode(bin_name, descriptors[mode].subcommands, argc, argv, cmdname_accumulator); } return NULL; @@ -173,10 +186,12 @@ setup_help_context(const gchar *cmdname, CommandDescriptor *active_mode) if (!active_mode) return NULL; - GOptionContext *ctx = g_option_context_new(cmdname); + GOptionContext *ctx = g_option_context_new(cmdname ? : active_mode->mode); g_option_context_set_summary(ctx, active_mode->description); g_option_context_add_main_entries(ctx, active_mode->options, NULL); g_option_context_add_main_entries(ctx, slng_options, NULL); + // Further detailed descriptions might go to the end of the list + //g_option_context_set_description(ctx, active_mode->detailed_description); return ctx; } @@ -191,15 +206,16 @@ main(int argc, char *argv[]) control_name = get_installation_path_for(PATH_CONTROL_SOCKET); - GString *cmdname_accumulator = g_string_new(argv[0]); - CommandDescriptor *active_mode = find_active_mode(modes, &argc, argv, cmdname_accumulator); - GOptionContext *ctx = setup_help_context(cmdname_accumulator->str, active_mode); + GString *bin_name = g_string_new(g_path_get_basename(argv[0])); + GString *cmdname_accumulator = g_string_new(NULL); + CommandDescriptor *active_mode = find_active_mode(bin_name, modes, &argc, argv, cmdname_accumulator); + GOptionContext *ctx = setup_help_context(cmdname_accumulator->len > 0 ? cmdname_accumulator->str : NULL, active_mode); g_string_free(cmdname_accumulator, TRUE); if (!ctx) { - fprintf(stderr, "Unknown command\n"); - print_usage(argv[0], modes); + fprintf(stderr, "Unknown command\n\n"); + print_usage(bin_name->str, "", modes); exit(1); } From 21b23c03aec9156af325ddda4bdf86d495a11c70 Mon Sep 17 00:00:00 2001 From: Hofi Date: Fri, 21 Feb 2025 23:49:25 +0100 Subject: [PATCH 3/9] console: add support for restoring the original console io handlers Signed-off-by: Hofi --- lib/console.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/console.c b/lib/console.c index da5a919d18..026d5d1d3f 100644 --- a/lib/console.c +++ b/lib/console.c @@ -32,6 +32,7 @@ GMutex console_lock; gboolean console_present = TRUE; gboolean using_initial_console = TRUE; const gchar *console_prefix; +gint initial_console_fds[3]; /** * console_printf: @@ -93,6 +94,10 @@ console_acquire_from_fds(gint fds[3]) (void) write(1, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); } + initial_console_fds[0] = dup(STDIN_FILENO); + initial_console_fds[1] = dup(STDOUT_FILENO); + initial_console_fds[2] = dup(STDERR_FILENO); + dup2(fds[0], STDIN_FILENO); dup2(fds[1], STDOUT_FILENO); dup2(fds[2], STDERR_FILENO); @@ -121,25 +126,27 @@ console_release(void) if (!console_present) goto exit; - devnull_fd = open("/dev/null", O_RDONLY); - if (devnull_fd >= 0) + if (initial_console_fds[0] > 0) { - dup2(devnull_fd, STDIN_FILENO); - close(devnull_fd); + dup2(initial_console_fds[0], STDIN_FILENO); + close(initial_console_fds[0]); + initial_console_fds[0] = -1; } - devnull_fd = open("/dev/null", O_WRONLY); - if (devnull_fd >= 0) + if (initial_console_fds[1] > 0) { - dup2(devnull_fd, STDOUT_FILENO); - dup2(devnull_fd, STDERR_FILENO); - close(devnull_fd); + dup2(initial_console_fds[1], STDOUT_FILENO); + close(initial_console_fds[1]); + initial_console_fds[1] = -1; + } + if (initial_console_fds[2] > 0) + { + dup2(initial_console_fds[2], STDERR_FILENO); + close(initial_console_fds[2]); + initial_console_fds[2] = -1; } - clearerr(stdin); - clearerr(stdout); - clearerr(stderr); - console_present = FALSE; - using_initial_console = FALSE; + console_present = TRUE; + using_initial_console = TRUE; exit: g_mutex_unlock(&console_lock); } @@ -149,6 +156,9 @@ console_global_init(const gchar *console_prefix_) { g_mutex_init(&console_lock); console_prefix = console_prefix_; + initial_console_fds[0] = -1; + initial_console_fds[1] = -1; + initial_console_fds[2] = -1; } void From a00406724fb391e28dc47b08d2d6b52aaaba7dc7 Mon Sep 17 00:00:00 2001 From: Hofi Date: Mon, 24 Feb 2025 20:25:09 +0100 Subject: [PATCH 4/9] console: remove the now unused console_present, use only using_initial_console instead Signed-off-by: Hofi --- lib/console.c | 22 ++++++++-------------- lib/console.h | 2 +- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/console.c b/lib/console.c index 026d5d1d3f..2b84e93a75 100644 --- a/lib/console.c +++ b/lib/console.c @@ -29,7 +29,6 @@ #include GMutex console_lock; -gboolean console_present = TRUE; gboolean using_initial_console = TRUE; const gchar *console_prefix; gint initial_console_fds[3]; @@ -55,7 +54,7 @@ console_printf(const gchar *fmt, ...) va_start(ap, fmt); g_vsnprintf(buf, sizeof(buf), fmt, ap); va_end(ap); - if (console_is_present()) + if (console_is_initial()) fprintf(stderr, "%s: %s\n", console_prefix, buf); else { @@ -68,13 +67,13 @@ console_printf(const gchar *fmt, ...) /* NOTE: this is not synced with any changes and is just an indication whether we have a console */ gboolean -console_is_present(void) +console_is_initial(void) { gboolean result; /* the lock only serves a memory barrier but is not a real synchronization */ g_mutex_lock(&console_lock); - result = console_present; + result = using_initial_console; g_mutex_unlock(&console_lock); return result; } @@ -87,12 +86,10 @@ console_acquire_from_fds(gint fds[3]) gboolean result = FALSE; g_mutex_lock(&console_lock); - if (console_present) - { - if (!using_initial_console) - goto exit; - (void) write(1, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); - } + if (using_initial_console) + goto exit; + + (void) write(1, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); initial_console_fds[0] = dup(STDIN_FILENO); initial_console_fds[1] = dup(STDOUT_FILENO); @@ -102,7 +99,6 @@ console_acquire_from_fds(gint fds[3]) dup2(fds[1], STDOUT_FILENO); dup2(fds[2], STDERR_FILENO); - console_present = TRUE; using_initial_console = FALSE; result = TRUE; exit: @@ -123,7 +119,7 @@ console_release(void) g_mutex_lock(&console_lock); - if (!console_present) + if (using_initial_console) goto exit; if (initial_console_fds[0] > 0) @@ -144,8 +140,6 @@ console_release(void) close(initial_console_fds[2]); initial_console_fds[2] = -1; } - - console_present = TRUE; using_initial_console = TRUE; exit: g_mutex_unlock(&console_lock); diff --git a/lib/console.h b/lib/console.h index c65347357b..4848495280 100644 --- a/lib/console.h +++ b/lib/console.h @@ -29,7 +29,7 @@ void console_printf(const gchar *fmt, ...) __attribute__ ((format (printf, 1, 2))); -gboolean console_is_present(void); +gboolean console_is_initial(void); gboolean console_acquire_from_fds(gint fds[3]); void console_release(void); From 87802779f710968c0380d17e19a3f99c1f6bfcda Mon Sep 17 00:00:00 2001 From: Hofi Date: Mon, 24 Feb 2025 21:32:28 +0100 Subject: [PATCH 5/9] console: added `--fds-to-steel` option to select the stdio file handlers to redirect on attach Signed-off-by: Hofi --- lib/console.c | 68 +++++++++++++----- lib/console.h | 2 +- lib/mainloop-control.c | 121 ++++++++++++++++++++------------ syslog-ng-ctl/commands/attach.c | 53 ++++++++++++-- 4 files changed, 177 insertions(+), 67 deletions(-) diff --git a/lib/console.c b/lib/console.c index 2b84e93a75..1d0d8a076d 100644 --- a/lib/console.c +++ b/lib/console.c @@ -32,6 +32,7 @@ GMutex console_lock; gboolean using_initial_console = TRUE; const gchar *console_prefix; gint initial_console_fds[3]; +gint stolen_fds; /** * console_printf: @@ -65,7 +66,7 @@ console_printf(const gchar *fmt, ...) } -/* NOTE: this is not synced with any changes and is just an indication whether we have a console */ +/* NOTE: this is not synced with any changes and is just an indication whether we already acquired the console */ gboolean console_is_initial(void) { @@ -78,26 +79,61 @@ console_is_initial(void) return result; } +GString * +_get_fn_names(gint fns) +{ + GString *result = g_string_new(NULL); + + if (fns & (1 << STDIN_FILENO)) + g_string_append(result, "stdin"); + if (fns & (1 << STDOUT_FILENO)) + { + if (result->len > 0) + g_string_append_c(result, ','); + g_string_append(result, "stdout"); + } + if (fns & (1 << STDERR_FILENO)) + { + if (result->len > 0) + g_string_append_c(result, ','); + g_string_append(result, "stderr"); + } + + return result; +} + /* re-acquire a console after startup using an array of fds */ gboolean -console_acquire_from_fds(gint fds[3]) +console_acquire_from_fds(gint fds[3], gint fds_to_steel) { - const gchar *takeover_message_on_old_console = "[Console taken over, no further output here]\n"; gboolean result = FALSE; g_mutex_lock(&console_lock); - if (using_initial_console) - goto exit; - - (void) write(1, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); - - initial_console_fds[0] = dup(STDIN_FILENO); - initial_console_fds[1] = dup(STDOUT_FILENO); - initial_console_fds[2] = dup(STDERR_FILENO); + if (!using_initial_console) + goto exit; - dup2(fds[0], STDIN_FILENO); - dup2(fds[1], STDOUT_FILENO); - dup2(fds[2], STDERR_FILENO); + GString *stolen_fn_names = _get_fn_names(fds_to_steel); + gchar *takeover_message_on_old_console = g_strdup_printf("[Console(%s) taken over, no further output here]\n", + stolen_fn_names->str); + (void) write(STDOUT_FILENO, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); + g_free(takeover_message_on_old_console); + g_string_free(stolen_fn_names, TRUE); + + stolen_fds = fds_to_steel; + + if (stolen_fds & (1 << STDIN_FILENO)) + initial_console_fds[0] = dup(STDIN_FILENO); + if (stolen_fds & (1 << STDOUT_FILENO)) + initial_console_fds[1] = dup(STDOUT_FILENO); + if (stolen_fds & (1 << STDERR_FILENO)) + initial_console_fds[2] = dup(STDERR_FILENO); + + if (stolen_fds & (1 << STDIN_FILENO)) + dup2(fds[0], STDIN_FILENO); + if (stolen_fds & (1 << STDOUT_FILENO)) + dup2(fds[1], STDOUT_FILENO); + if (stolen_fds & (1 << STDERR_FILENO)) + dup2(fds[2], STDERR_FILENO); using_initial_console = FALSE; result = TRUE; @@ -109,14 +145,12 @@ console_acquire_from_fds(gint fds[3]) /** * console_release: * - * Use /dev/null as input/output/error. This function is idempotent, can be + * Restore input/output/error. This function is idempotent, can be * called any number of times without harm. **/ void console_release(void) { - gint devnull_fd; - g_mutex_lock(&console_lock); if (using_initial_console) diff --git a/lib/console.h b/lib/console.h index 4848495280..f1a218f35c 100644 --- a/lib/console.h +++ b/lib/console.h @@ -30,7 +30,7 @@ void console_printf(const gchar *fmt, ...) __attribute__ ((format (printf, 1, 2))); gboolean console_is_initial(void); -gboolean console_acquire_from_fds(gint fds[3]); +gboolean console_acquire_from_fds(gint fds[3], gint fds_to_steel); void console_release(void); void console_global_init(const gchar *console_prefix); diff --git a/lib/mainloop-control.c b/lib/mainloop-control.c index 6f33f81bb7..15ec0a9cca 100644 --- a/lib/mainloop-control.c +++ b/lib/mainloop-control.c @@ -35,6 +35,7 @@ #include "debugger/debugger-main.h" #include +#include static gboolean _control_process_log_level(const gchar *level, GString *result) @@ -119,26 +120,22 @@ _wait_until_peer_disappears(ControlConnection *cc, gint max_seconds, gboolean *c console_release(); } -static void -control_connection_attach(ControlConnection *cc, GString *command, gpointer user_data, gboolean *cancelled) +typedef struct _AttachCommandArgs { - MainLoop *main_loop = (MainLoop *) user_data; - gchar **cmds = g_strsplit(command->str, " ", 4); + gboolean start_debugger; + gint fds_to_steel; + gint n_seconds; + gboolean log_stderr; + gint log_level; +} AttachCommandArgs; - GString *result = g_string_sized_new(128); - gint n_seconds = -1; - gboolean start_debugger = FALSE; - struct - { - gboolean log_stderr; - gint log_level; - } old_values, new_values; - - old_values.log_stderr = log_stderr; - old_values.log_level = msg_get_log_level(); - new_values = old_values; +static gboolean +_parse_attach_command_args(GString *command, AttachCommandArgs *args, GString *result) +{ + gboolean success = FALSE; + gchar **cmds = g_strsplit(command->str, " ", 5); - if (!cmds[1]) + if (cmds[1] == NULL) { g_string_assign(result, "FAIL Invalid arguments received"); goto exit; @@ -146,22 +143,24 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user if (g_str_equal(cmds[1], "STDIO")) { - ; + if (cmds[3]) + args->fds_to_steel = atoi(cmds[3]); } else if (g_str_equal(cmds[1], "LOGS")) { - new_values.log_stderr = TRUE; - if (cmds[3]) - new_values.log_level = msg_map_string_to_log_level(cmds[3]); - if (new_values.log_level < 0) - { - g_string_assign(result, "FAIL Invalid log level"); - goto exit; - } + args->log_stderr = TRUE; + /* NOTE: as log_stderr uses stderr (what a surprise) + * - we need to steal only the stderr + * - caller of the `attach logs` will get the stderr output as well, so + * they should redirect stderr to stdout if they want to capture it for + * further processing e.g. + * syslog-ng-ctl attach logs |& grep -i error + */ + args->fds_to_steel = (1 << STDERR_FILENO); } else if (g_str_equal(cmds[1], "DEBUGGER")) { - start_debugger = TRUE; + args->start_debugger = TRUE; } else { @@ -170,49 +169,85 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user } if (cmds[2]) - n_seconds = atoi(cmds[2]); + args->n_seconds = atoi(cmds[2]); + + if (cmds[4]) + args->log_level = msg_map_string_to_log_level(cmds[4]); + if (args->log_level < 0) + { + g_string_assign(result, "FAIL Invalid log level"); + goto exit; + } + success = TRUE; + +exit: + g_strfreev(cmds); + return success; +} + +static void +control_connection_attach(ControlConnection *cc, GString *command, gpointer user_data, gboolean *cancelled) +{ + MainLoop *main_loop = (MainLoop *) user_data; + GString *result = g_string_sized_new(128); + struct + { + gboolean log_stderr; + gint log_level; + } old_values = + { + log_stderr, + msg_get_log_level() + }; + AttachCommandArgs cmd_args = + { + .start_debugger = FALSE, + .fds_to_steel = (1 << STDOUT_FILENO) | (1 << STDERR_FILENO), + .n_seconds = -1, + .log_stderr = old_values.log_stderr, + .log_level = old_values.log_level + }; + + if (FALSE == _parse_attach_command_args(command, &cmd_args, result)) + goto exit; gint fds[3]; gsize num_fds = G_N_ELEMENTS(fds); - if (!control_connection_get_attached_fds(cc, fds, &num_fds) || num_fds != 3) + if (FALSE == control_connection_get_attached_fds(cc, fds, &num_fds) || num_fds != 3) { g_string_assign(result, "FAIL The underlying transport for syslog-ng-ctl does not support fd passing or incorrect number of fds received"); goto exit; } - if (!console_acquire_from_fds(fds)) + if (FALSE == console_acquire_from_fds(fds, cmd_args.fds_to_steel)) { - g_string_assign(result, - "FAIL Error acquiring console"); + g_string_assign(result, "FAIL Error acquiring console"); goto exit; } - log_stderr = new_values.log_stderr; - msg_set_log_level(new_values.log_level); + log_stderr = cmd_args.log_stderr; + msg_set_log_level(cmd_args.log_level); - if (start_debugger && !debugger_is_running()) + if (cmd_args.start_debugger && FALSE == debugger_is_running()) { //cfg_load_module(self->current_configuration, "mod-python"); debugger_start(main_loop, main_loop_get_current_config(main_loop)); } - _wait_until_peer_disappears(cc, n_seconds, cancelled); + _wait_until_peer_disappears(cc, cmd_args.n_seconds, cancelled); - if (start_debugger && debugger_is_running()) - { - debugger_stop(); - } + if (cmd_args.start_debugger && debugger_is_running()) + debugger_stop(); log_stderr = old_values.log_stderr; msg_set_log_level(old_values.log_level); - g_string_assign(result, "OK [console output ends here]"); -exit: + g_string_assign(result, "OK [Console output ends here]"); +exit: control_connection_send_batched_reply(cc, result); control_connection_send_close_batch(cc); - g_strfreev(cmds); } static void diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c index 2bd18ac246..de50b22f59 100644 --- a/syslog-ng-ctl/commands/attach.c +++ b/syslog-ng-ctl/commands/attach.c @@ -24,8 +24,11 @@ #include "ctl-stats.h" #include "syslog-ng.h" -static gint attach_options_seconds; +#include + +static gint attach_options_seconds = -1; static gchar *attach_options_log_level = NULL; +static gint attach_options_fds_to_steel = 0; static gchar **attach_commands = NULL; static gboolean @@ -43,10 +46,39 @@ _store_log_level(const gchar *option_name, return FALSE; } +static gboolean +_parse_fd_names(const gchar *option_name, + const gchar *value, + gpointer data, + GError **error) +{ + gboolean result = TRUE; + gchar **fds = g_strsplit(value, ",", 3); + + attach_options_fds_to_steel = 0; + for (gchar **fd = fds; *fd; fd++) + { + if (g_str_equal(*fd, "stdin")) + attach_options_fds_to_steel |= (1 << STDIN_FILENO); + else if (g_str_equal(*fd, "stdout")) + attach_options_fds_to_steel |= (1 << STDOUT_FILENO); + else if (g_str_equal(*fd, "stderr")) + attach_options_fds_to_steel |= (1 << STDERR_FILENO); + else + { + g_set_error(error, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, "Unknown %s option value: %s", option_name, *fd); + result = FALSE; + } + } + g_strfreev(fds); + return result; +} + GOptionEntry attach_options[] = { - { "seconds", 0, 0, G_OPTION_ARG_INT, &attach_options_seconds, "amount of time to attach for", NULL }, - { "log-level", 0, 0, G_OPTION_ARG_CALLBACK, _store_log_level, "change syslog-ng log level", "" }, + { "seconds", 's', 0, G_OPTION_ARG_INT, &attach_options_seconds, "amount of time to attach for", NULL }, + { "log-level", 'l', 0, G_OPTION_ARG_CALLBACK, _store_log_level, "change syslog-ng log level", "" }, + { "fds-to-steel", 'f', 0, G_OPTION_ARG_CALLBACK, _parse_fd_names, "which stdio file handlers to attach to, default is , valid only with the `stdio` attach mode", " in a comma separated list" }, { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &attach_commands, "attach mode: logs, debugger, stdio", NULL }, { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL } }; @@ -61,7 +93,7 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) { if (attach_commands[1]) { - fprintf(stderr, "Too many arguments"); + fprintf(stderr, "Error parsing command line arguments: Too many arguments"); return 1; } attach_mode = attach_commands[0]; @@ -77,13 +109,22 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) g_string_append(command, " DEBUGGER"); else { - fprintf(stderr, "Unknown attach mode\n"); + fprintf(stderr, "Error parsing command line arguments: Unknown attach mode\n"); + return 1; + } + if (FALSE == g_str_equal(attach_mode, "stdio") && attach_options_fds_to_steel > 0) + { + fprintf(stderr, "Error parsing command line arguments: %s is valid only with %s attach mode\n", + "fds-to-steel", "stdio"); return 1; } - g_string_append_printf(command, " %d", attach_options_seconds ? : -1); + g_string_append_printf(command, " %d", attach_options_seconds > 0 ? attach_options_seconds : -1); + g_string_append_printf(command, " %d", + attach_options_fds_to_steel > 0 ? attach_options_fds_to_steel : (1 << STDOUT_FILENO) | (1 << STDERR_FILENO)); if (attach_options_log_level) g_string_append_printf(command, " %s", attach_options_log_level); + gint result = attach_command(command->str); g_string_free(command, TRUE); return result; From bb7da311ae2fe4d08a8b435c90de6cf0c1c38fb0 Mon Sep 17 00:00:00 2001 From: Hofi Date: Mon, 24 Feb 2025 23:13:57 +0100 Subject: [PATCH 6/9] syslog-ng-ctl-attach: use proper command+sub_command helps to get accurate help contexts and help info Signed-off-by: Hofi --- syslog-ng-ctl/commands/attach.c | 52 +++++++++++++++++-------------- syslog-ng-ctl/commands/attach.h | 3 +- syslog-ng-ctl/commands/commands.h | 4 +++ syslog-ng-ctl/syslog-ng-ctl.c | 6 ++-- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c index de50b22f59..f2bb1b9a3e 100644 --- a/syslog-ng-ctl/commands/attach.c +++ b/syslog-ng-ctl/commands/attach.c @@ -21,7 +21,7 @@ * */ -#include "ctl-stats.h" +#include "commands.h" #include "syslog-ng.h" #include @@ -29,7 +29,6 @@ static gint attach_options_seconds = -1; static gchar *attach_options_log_level = NULL; static gint attach_options_fds_to_steel = 0; -static gchar **attach_commands = NULL; static gboolean _store_log_level(const gchar *option_name, @@ -74,32 +73,11 @@ _parse_fd_names(const gchar *option_name, return result; } -GOptionEntry attach_options[] = -{ - { "seconds", 's', 0, G_OPTION_ARG_INT, &attach_options_seconds, "amount of time to attach for", NULL }, - { "log-level", 'l', 0, G_OPTION_ARG_CALLBACK, _store_log_level, "change syslog-ng log level", "" }, - { "fds-to-steel", 'f', 0, G_OPTION_ARG_CALLBACK, _parse_fd_names, "which stdio file handlers to attach to, default is , valid only with the `stdio` attach mode", " in a comma separated list" }, - { G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &attach_commands, "attach mode: logs, debugger, stdio", NULL }, - { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL } -}; - gint slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) { GString *command = g_string_new("ATTACH"); - const gchar *attach_mode; - - if (attach_commands) - { - if (attach_commands[1]) - { - fprintf(stderr, "Error parsing command line arguments: Too many arguments"); - return 1; - } - attach_mode = attach_commands[0]; - } - else - attach_mode = "stdio"; + const gchar *attach_mode = mode ? : "stdio"; if (g_str_equal(attach_mode, "stdio")) g_string_append(command, " STDIO"); @@ -129,3 +107,29 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) g_string_free(command, TRUE); return result; } + +#define SECONDS_OPTION_ENTRY OPTIONS_ENTRY("seconds", 's', 0, G_OPTION_ARG_INT, &attach_options_seconds, "amount of time to attach for", NULL) +#define LOG_LEVEL_OPTION_ENTRY OPTIONS_ENTRY("log-level", 'l', 0, G_OPTION_ARG_CALLBACK, _store_log_level, "change syslog-ng log level", "") + +const GOptionEntry attach_stdio_options[] = +{ + SECONDS_OPTION_ENTRY, + LOG_LEVEL_OPTION_ENTRY, + { "fds-to-steel", 'f', 0, G_OPTION_ARG_CALLBACK, _parse_fd_names, "which stdio file handlers to attach to, default is ", " in a comma separated list" }, + { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL } +}; + +GOptionEntry attach_logs_and_debugger_options[] = +{ + SECONDS_OPTION_ENTRY, + LOG_LEVEL_OPTION_ENTRY, + { NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL } +}; + +CommandDescriptor attach_commands[] = +{ + { "stdio", attach_stdio_options, "Attach to syslog-ng console using the given file handlers", slng_attach }, + { "logs", attach_logs_and_debugger_options, "Attach to syslog-ng internal logs, which are normally redirected via stderr; for further processing, use another redirection, e.g., `syslog-ng-ctl attach logs |& grep -i error.`", slng_attach }, + { "debugger", attach_logs_and_debugger_options, "Start and attach to syslog-ng debugger", slng_attach }, + { NULL, NULL, NULL, NULL } +}; diff --git a/syslog-ng-ctl/commands/attach.h b/syslog-ng-ctl/commands/attach.h index b544924ff4..540c02efa3 100644 --- a/syslog-ng-ctl/commands/attach.h +++ b/syslog-ng-ctl/commands/attach.h @@ -26,7 +26,6 @@ #include "commands.h" -extern GOptionEntry attach_options[]; -gint slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx); +extern CommandDescriptor attach_commands[]; #endif diff --git a/syslog-ng-ctl/commands/commands.h b/syslog-ng-ctl/commands/commands.h index f24c4f586a..4af1ab616e 100644 --- a/syslog-ng-ctl/commands/commands.h +++ b/syslog-ng-ctl/commands/commands.h @@ -29,6 +29,10 @@ #include +/* Though clang has no issues, but gcc is stricter about requiring initializer elements to be compile-time constants when initializing structures directly + * This helper macro is used to initialize GOptionEntry structures with the same values as the GOptionEntry initializers without having to repeat the initializer values */ +#define OPTIONS_ENTRY(long_name, short_name, flags, arg, arg_data, description, arg_description) { long_name, short_name, flags, arg, arg_data, description, arg_description } + extern GOptionEntry no_options[]; typedef struct _CommandDescriptor diff --git a/syslog-ng-ctl/syslog-ng-ctl.c b/syslog-ng-ctl/syslog-ng-ctl.c index 85ad822c68..f01b028b3d 100644 --- a/syslog-ng-ctl/syslog-ng-ctl.c +++ b/syslog-ng-ctl/syslog-ng-ctl.c @@ -112,7 +112,8 @@ slng_export_config_graph(int argc, char *argv[], const gchar *mode, GOptionConte static CommandDescriptor modes[] = { - { "attach", attach_options, "Attach to a running syslog-ng instance", slng_attach, NULL }, + { "attach", no_options, "Attach to a running syslog-ng instance", NULL, attach_commands }, + // TODO: Add proper details of the sub-commands like attach and credentials have { "stats", stats_options, "Get syslog-ng statistics. Possible commands: csv, prometheus; default: csv", slng_stats, NULL }, { "verbose", verbose_options, "Enable/query verbose messages", slng_verbose, NULL }, { "debug", verbose_options, "Enable/query debug messages", slng_verbose, NULL }, @@ -121,6 +122,7 @@ static CommandDescriptor modes[] = { "stop", no_options, "Stop syslog-ng process", slng_stop, NULL }, { "reload", no_options, "Reload syslog-ng", slng_reload, NULL }, { "reopen", no_options, "Re-open of log destination files", slng_reopen, NULL }, + // TODO: Add proper details of the sub-commands like attach and credentials have { "query", query_options, "Query syslog-ng statistics. Possible commands: list, get, get --sum", slng_query, NULL }, { "show-license-info", license_options, "Show information about the license", slng_license, NULL }, { "credentials", no_options, "Credentials manager", NULL, credentials_commands }, @@ -190,8 +192,6 @@ setup_help_context(const gchar *cmdname, CommandDescriptor *active_mode) g_option_context_set_summary(ctx, active_mode->description); g_option_context_add_main_entries(ctx, active_mode->options, NULL); g_option_context_add_main_entries(ctx, slng_options, NULL); - // Further detailed descriptions might go to the end of the list - //g_option_context_set_description(ctx, active_mode->detailed_description); return ctx; } From dba7e9cc02e7143ea463afe57adda700e95c1a57 Mon Sep 17 00:00:00 2001 From: Hofi Date: Wed, 26 Feb 2025 12:35:37 +0100 Subject: [PATCH 7/9] style: removed usage of FALSE == as requested Signed-off-by: Hofi --- lib/mainloop-control.c | 8 ++++---- syslog-ng-ctl/commands/attach.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/mainloop-control.c b/lib/mainloop-control.c index 15ec0a9cca..ec5027810d 100644 --- a/lib/mainloop-control.c +++ b/lib/mainloop-control.c @@ -208,19 +208,19 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user .log_level = old_values.log_level }; - if (FALSE == _parse_attach_command_args(command, &cmd_args, result)) + if (!_parse_attach_command_args(command, &cmd_args, result)) goto exit; gint fds[3]; gsize num_fds = G_N_ELEMENTS(fds); - if (FALSE == control_connection_get_attached_fds(cc, fds, &num_fds) || num_fds != 3) + if (!control_connection_get_attached_fds(cc, fds, &num_fds) || num_fds != 3) { g_string_assign(result, "FAIL The underlying transport for syslog-ng-ctl does not support fd passing or incorrect number of fds received"); goto exit; } - if (FALSE == console_acquire_from_fds(fds, cmd_args.fds_to_steel)) + if (!console_acquire_from_fds(fds, cmd_args.fds_to_steel)) { g_string_assign(result, "FAIL Error acquiring console"); goto exit; @@ -229,7 +229,7 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user log_stderr = cmd_args.log_stderr; msg_set_log_level(cmd_args.log_level); - if (cmd_args.start_debugger && FALSE == debugger_is_running()) + if (cmd_args.start_debugger && !debugger_is_running()) { //cfg_load_module(self->current_configuration, "mod-python"); debugger_start(main_loop, main_loop_get_current_config(main_loop)); diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c index f2bb1b9a3e..3f314544f7 100644 --- a/syslog-ng-ctl/commands/attach.c +++ b/syslog-ng-ctl/commands/attach.c @@ -90,7 +90,7 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) fprintf(stderr, "Error parsing command line arguments: Unknown attach mode\n"); return 1; } - if (FALSE == g_str_equal(attach_mode, "stdio") && attach_options_fds_to_steel > 0) + if (!g_str_equal(attach_mode, "stdio") && attach_options_fds_to_steel > 0) { fprintf(stderr, "Error parsing command line arguments: %s is valid only with %s attach mode\n", "fds-to-steel", "stdio"); From 70fc851b3e87d54585b823dbcea5fdf758300e1d Mon Sep 17 00:00:00 2001 From: Hofi Date: Thu, 27 Feb 2025 13:25:45 +0100 Subject: [PATCH 8/9] console: spelling fix Signed-off-by: Hofi # Conflicts: # lib/mainloop-control.c # syslog-ng-ctl/commands/attach.c Signed-off-by: Hofi --- lib/console.c | 6 +++--- lib/console.h | 2 +- lib/mainloop-control.c | 10 +++++----- syslog-ng-ctl/commands/attach.c | 14 +++++++------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/console.c b/lib/console.c index 1d0d8a076d..dc6b0f2f76 100644 --- a/lib/console.c +++ b/lib/console.c @@ -104,7 +104,7 @@ _get_fn_names(gint fns) /* re-acquire a console after startup using an array of fds */ gboolean -console_acquire_from_fds(gint fds[3], gint fds_to_steel) +console_acquire_from_fds(gint fds[3], gint fds_to_steal) { gboolean result = FALSE; @@ -112,14 +112,14 @@ console_acquire_from_fds(gint fds[3], gint fds_to_steel) if (!using_initial_console) goto exit; - GString *stolen_fn_names = _get_fn_names(fds_to_steel); + GString *stolen_fn_names = _get_fn_names(fds_to_steal); gchar *takeover_message_on_old_console = g_strdup_printf("[Console(%s) taken over, no further output here]\n", stolen_fn_names->str); (void) write(STDOUT_FILENO, takeover_message_on_old_console, strlen(takeover_message_on_old_console)); g_free(takeover_message_on_old_console); g_string_free(stolen_fn_names, TRUE); - stolen_fds = fds_to_steel; + stolen_fds = fds_to_steal; if (stolen_fds & (1 << STDIN_FILENO)) initial_console_fds[0] = dup(STDIN_FILENO); diff --git a/lib/console.h b/lib/console.h index f1a218f35c..7613eee498 100644 --- a/lib/console.h +++ b/lib/console.h @@ -30,7 +30,7 @@ void console_printf(const gchar *fmt, ...) __attribute__ ((format (printf, 1, 2))); gboolean console_is_initial(void); -gboolean console_acquire_from_fds(gint fds[3], gint fds_to_steel); +gboolean console_acquire_from_fds(gint fds[3], gint fds_to_steal); void console_release(void); void console_global_init(const gchar *console_prefix); diff --git a/lib/mainloop-control.c b/lib/mainloop-control.c index ec5027810d..56231d463f 100644 --- a/lib/mainloop-control.c +++ b/lib/mainloop-control.c @@ -123,7 +123,7 @@ _wait_until_peer_disappears(ControlConnection *cc, gint max_seconds, gboolean *c typedef struct _AttachCommandArgs { gboolean start_debugger; - gint fds_to_steel; + gint fds_to_steal; gint n_seconds; gboolean log_stderr; gint log_level; @@ -144,7 +144,7 @@ _parse_attach_command_args(GString *command, AttachCommandArgs *args, GString *r if (g_str_equal(cmds[1], "STDIO")) { if (cmds[3]) - args->fds_to_steel = atoi(cmds[3]); + args->fds_to_steal = atoi(cmds[3]); } else if (g_str_equal(cmds[1], "LOGS")) { @@ -156,7 +156,7 @@ _parse_attach_command_args(GString *command, AttachCommandArgs *args, GString *r * further processing e.g. * syslog-ng-ctl attach logs |& grep -i error */ - args->fds_to_steel = (1 << STDERR_FILENO); + args->fds_to_steal = (1 << STDERR_FILENO); } else if (g_str_equal(cmds[1], "DEBUGGER")) { @@ -202,7 +202,7 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user AttachCommandArgs cmd_args = { .start_debugger = FALSE, - .fds_to_steel = (1 << STDOUT_FILENO) | (1 << STDERR_FILENO), + .fds_to_steal = (1 << STDOUT_FILENO) | (1 << STDERR_FILENO), .n_seconds = -1, .log_stderr = old_values.log_stderr, .log_level = old_values.log_level @@ -220,7 +220,7 @@ control_connection_attach(ControlConnection *cc, GString *command, gpointer user goto exit; } - if (!console_acquire_from_fds(fds, cmd_args.fds_to_steel)) + if (!console_acquire_from_fds(fds, cmd_args.fds_to_steal)) { g_string_assign(result, "FAIL Error acquiring console"); goto exit; diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c index 3f314544f7..5a044d2e57 100644 --- a/syslog-ng-ctl/commands/attach.c +++ b/syslog-ng-ctl/commands/attach.c @@ -28,7 +28,7 @@ static gint attach_options_seconds = -1; static gchar *attach_options_log_level = NULL; -static gint attach_options_fds_to_steel = 0; +static gint attach_options_fds_to_steal = 0; static gboolean _store_log_level(const gchar *option_name, @@ -54,15 +54,15 @@ _parse_fd_names(const gchar *option_name, gboolean result = TRUE; gchar **fds = g_strsplit(value, ",", 3); - attach_options_fds_to_steel = 0; + attach_options_fds_to_steal = 0; for (gchar **fd = fds; *fd; fd++) { if (g_str_equal(*fd, "stdin")) - attach_options_fds_to_steel |= (1 << STDIN_FILENO); + attach_options_fds_to_steal |= (1 << STDIN_FILENO); else if (g_str_equal(*fd, "stdout")) - attach_options_fds_to_steel |= (1 << STDOUT_FILENO); + attach_options_fds_to_steal |= (1 << STDOUT_FILENO); else if (g_str_equal(*fd, "stderr")) - attach_options_fds_to_steel |= (1 << STDERR_FILENO); + attach_options_fds_to_steal |= (1 << STDERR_FILENO); else { g_set_error(error, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, "Unknown %s option value: %s", option_name, *fd); @@ -90,7 +90,7 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) fprintf(stderr, "Error parsing command line arguments: Unknown attach mode\n"); return 1; } - if (!g_str_equal(attach_mode, "stdio") && attach_options_fds_to_steel > 0) + if (!g_str_equal(attach_mode, "stdio") && attach_options_fds_to_steal > 0) { fprintf(stderr, "Error parsing command line arguments: %s is valid only with %s attach mode\n", "fds-to-steel", "stdio"); @@ -99,7 +99,7 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) g_string_append_printf(command, " %d", attach_options_seconds > 0 ? attach_options_seconds : -1); g_string_append_printf(command, " %d", - attach_options_fds_to_steel > 0 ? attach_options_fds_to_steel : (1 << STDOUT_FILENO) | (1 << STDERR_FILENO)); + attach_options_fds_to_steal > 0 ? attach_options_fds_to_steal : (1 << STDOUT_FILENO) | (1 << STDERR_FILENO)); if (attach_options_log_level) g_string_append_printf(command, " %s", attach_options_log_level); From 6ff5cc5d7cdb3cfdc053988df02251611e8413f7 Mon Sep 17 00:00:00 2001 From: Hofi Date: Thu, 27 Feb 2025 22:30:15 +0100 Subject: [PATCH 9/9] syslog-ng-ctl: attach uses the automatic GLib Commandline Option Parser validation, sub-commands and options do not need to be double validated Signed-off-by: Hofi --- syslog-ng-ctl/commands/attach.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/syslog-ng-ctl/commands/attach.c b/syslog-ng-ctl/commands/attach.c index 5a044d2e57..290b2f9def 100644 --- a/syslog-ng-ctl/commands/attach.c +++ b/syslog-ng-ctl/commands/attach.c @@ -73,6 +73,8 @@ _parse_fd_names(const gchar *option_name, return result; } +/* NOTE: this attach command handler uses the normal, automatic GLib Commandline Option Parser + * to parse the sub-command arguments with options, so there is no need to manually validate the sub-command and the options */ gint slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) { @@ -85,17 +87,6 @@ slng_attach(int argc, char *argv[], const gchar *mode, GOptionContext *ctx) g_string_append(command, " LOGS"); else if (g_str_equal(attach_mode, "debugger")) g_string_append(command, " DEBUGGER"); - else - { - fprintf(stderr, "Error parsing command line arguments: Unknown attach mode\n"); - return 1; - } - if (!g_str_equal(attach_mode, "stdio") && attach_options_fds_to_steal > 0) - { - fprintf(stderr, "Error parsing command line arguments: %s is valid only with %s attach mode\n", - "fds-to-steel", "stdio"); - return 1; - } g_string_append_printf(command, " %d", attach_options_seconds > 0 ? attach_options_seconds : -1); g_string_append_printf(command, " %d",