diff --git a/include/option-parser.h b/include/option-parser.h index 098e2ee..03492e8 100644 --- a/include/option-parser.h +++ b/include/option-parser.h @@ -17,26 +17,40 @@ #pragma once #include -#include +#include struct wv_option { char short_opt; const char* long_opt; const char* schema; const char* help; + const char* default_; + const char* positional; + bool is_subcommand; +}; + +struct wv_option_value { + const struct wv_option* option; + char value[256]; }; struct option_parser { const struct wv_option* options; - char short_opts[128]; - struct option long_opts[128]; int n_opts; + + struct wv_option_value values[128]; + int n_values; + int position; + + int endpos; }; -#define OPTION_PARSER_CHECK_ALL_ARGUMENTS 0 -#define OPTION_PARSER_STOP_ON_FIRST_NONOPTION 1 - -void option_parser_init(struct option_parser* self, const struct wv_option* options, unsigned flags); +void option_parser_init(struct option_parser* self, + const struct wv_option* options); void option_parser_print_options(struct option_parser* self, FILE* stream); -int option_parser_getopt(struct option_parser* self, int argc, char* argv[]); + +int option_parser_parse(struct option_parser* self, int argc, + const char* const* argv); +const char* option_parser_get_value(const struct option_parser* self, + const char* name); diff --git a/meson.build b/meson.build index 157b368..3b11823 100644 --- a/meson.build +++ b/meson.build @@ -19,6 +19,7 @@ c_args = [ '-DAML_UNSTABLE_API=1', '-Wno-unused-parameter', + '-Wno-missing-field-initializers', ] git = find_program('git', native: true, required: false) @@ -197,3 +198,7 @@ if scdoc.found() ) endforeach endif + +if get_option('tests') + subdir('test') +endif diff --git a/meson_options.txt b/meson_options.txt index ad052a4..9b94654 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -6,3 +6,5 @@ option('man-pages', type: 'feature', value: 'auto', description: 'Generate and install man pages') option('systemtap', type: 'boolean', value: false, description: 'Enable tracing using sdt') +option('tests', type: 'boolean', value: false, + description: 'Build unit tests') diff --git a/src/main.c b/src/main.c index 304b42d..8ee7a15 100644 --- a/src/main.c +++ b/src/main.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -1069,7 +1068,7 @@ static void on_nvnc_client_new(struct nvnc_client* client) self->nr_clients); } -void parse_keyboard_option(struct wayvnc* self, char* arg) +void parse_keyboard_option(struct wayvnc* self, const char* arg) { // Find optional variant, separated by - char* index = strchr(arg, '-'); @@ -1252,6 +1251,7 @@ int main(int argc, char* argv[]) const char* output_name = NULL; const char* seat_name = NULL; const char* socket_path = NULL; + const char* keyboard_options = NULL; bool overlay_cursor = false; bool show_performance = false; @@ -1263,6 +1263,8 @@ int main(int argc, char* argv[]) int log_level = NVNC_LOG_WARNING; static const struct wv_option opts[] = { + { .positional = "address" }, + { .positional = "port" }, { 'C', "config", "", "Select a config file." }, { 'g', "gpu", NULL, @@ -1278,7 +1280,8 @@ int main(int argc, char* argv[]) { 'r', "render-cursor", NULL, "Enable overlay cursor rendering." }, { 'f', "max-fps", "", - "Set rate limit (default 30)." }, + "Set rate limit (default 30).", + .default_ = "30" }, { 'p', "performance", NULL, "Show performance counters." }, { 'u', "unix-socket", NULL, @@ -1290,84 +1293,53 @@ int main(int argc, char* argv[]) { 'v', "verbose", NULL, "Be more verbose. Same as setting --log-level=info" }, { 'L', "log-level", "", - "Set log level. The levels are: error, warning, info, debug trace and quiet." }, + "Set log level. The levels are: error, warning, info, debug trace and quiet.", + .default_ = "warning" }, { 'h', "help", NULL, "Get help (this text)." }, { '\0', NULL, NULL, NULL } }; struct option_parser option_parser; - option_parser_init(&option_parser, opts, - OPTION_PARSER_CHECK_ALL_ARGUMENTS); + option_parser_init(&option_parser, opts); - while (1) { - int c = option_parser_getopt(&option_parser, argc, argv); - if (c < 0) - break; + if (option_parser_parse(&option_parser, argc, + (const char* const*)argv) < 0) + return wayvnc_usage(&option_parser, stderr, 1); - switch (c) { - case 'C': - cfg_file = optarg; - break; - case 'g': - enable_gpu_features = true; - break; - case 'o': - output_name = optarg; - break; - case 'k': - parse_keyboard_option(&self, optarg); - break; - case 's': - seat_name = optarg; - break; - case 'S': - socket_path = optarg; - break; - case 'r': - overlay_cursor = true; - break; - case 'f': - max_rate = atoi(optarg); - break; - case 'p': - show_performance = true; - break; - case 'u': - use_unix_socket = true; - break; - case 'd': - disable_input = true; - break; - case 'v': - log_level = NVNC_LOG_INFO; - break; - case 'L': - log_level = log_level_from_string(optarg); - if (log_level < 0) { - fprintf(stderr, "Invalid log level: %s\n", - optarg); - return wayvnc_usage(&option_parser, stderr, 1); - } - break; - case 'V': - return show_version(); - case 'h': - return wayvnc_usage(&option_parser, stdout, 0); - default: - return wayvnc_usage(&option_parser, stderr, 1); - } + if (option_parser_get_value(&option_parser, "version")) { + return show_version(); } + if (option_parser_get_value(&option_parser, "help")) { + return wayvnc_usage(&option_parser, stdout, 0); + } + + cfg_file = option_parser_get_value(&option_parser, "config"); + enable_gpu_features = !!option_parser_get_value(&option_parser, "gpu"); + output_name = option_parser_get_value(&option_parser, "output"); + seat_name = option_parser_get_value(&option_parser, "seat"); + socket_path = option_parser_get_value(&option_parser, "socket"); + overlay_cursor = !!option_parser_get_value(&option_parser, "render-cursor"); + show_performance = !!option_parser_get_value(&option_parser, "performance"); + use_unix_socket = !!option_parser_get_value(&option_parser, "unix"); + disable_input = !!option_parser_get_value(&option_parser, "disable-input"); + log_level = option_parser_get_value(&option_parser, "verbose") + ? NVNC_LOG_INFO : NVNC_LOG_WARNING; + log_level = log_level_from_string( + option_parser_get_value(&option_parser, "log-level")); + max_rate = atoi(option_parser_get_value(&option_parser, "max-fps")); + + keyboard_options = option_parser_get_value(&option_parser, "keyboard"); + if (keyboard_options) + parse_keyboard_option(&self, keyboard_options); + nvnc_set_log_level(log_level); - int n_args = argc - optind; - - if (n_args >= 1) - address = argv[optind]; - - if (n_args >= 2) - port = atoi(argv[optind + 1]); + address = option_parser_get_value(&option_parser, "address"); + const char* port_str = option_parser_get_value(&option_parser, "port"); + if (port_str) + port = atoi(port_str); if (seat_name && disable_input) { nvnc_log(NVNC_LOG_ERROR, "seat and disable-input are conflicting options"); diff --git a/src/option-parser.c b/src/option-parser.c index 78d2a04..e11d522 100644 --- a/src/option-parser.c +++ b/src/option-parser.c @@ -15,53 +15,30 @@ */ #include "option-parser.h" +#include "strlcpy.h" #include #include #include #include +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) + static int count_options(const struct wv_option* opts) { int n = 0; - while (opts[n].short_opt || opts[n].long_opt) + while (opts[n].short_opt || opts[n].long_opt || opts[n].positional) n++; return n; } void option_parser_init(struct option_parser* self, - const struct wv_option* options, - unsigned flags) + const struct wv_option* options) { memset(self, 0, sizeof(*self)); self->options = options; self->n_opts = count_options(options); - - int short_opt_index = 0; - int long_opt_index = 0; - - if (flags && OPTION_PARSER_STOP_ON_FIRST_NONOPTION) - self->short_opts[short_opt_index++] = '+'; - - for (int i = 0; i < self->n_opts; ++i) { - assert(options[i].short_opt); // TODO: Make this optional? - - self->short_opts[short_opt_index++] = options[i].short_opt; - if (options[i].schema) - self->short_opts[short_opt_index++] = ':'; - - if (!options[i].long_opt) - continue; - - const struct wv_option* src_opt = &options[i]; - struct option* dst_opt = &self->long_opts[long_opt_index++]; - - dst_opt->val = src_opt->short_opt; - dst_opt->name = src_opt->long_opt; - dst_opt->has_arg = src_opt->schema ? - required_argument : no_argument; - } } static int get_left_col_width(const struct wv_option* opts, int n) @@ -129,7 +106,8 @@ static void reflow_text(char* dst, const char* src, int width) static void format_option(const struct wv_option* opt, int left_col_width, FILE* stream) { - assert(opt->help); + if (!opt->help) + return; int n_chars = fprintf(stream, " "); if (opt->short_opt) @@ -171,7 +149,223 @@ void option_parser_print_options(struct option_parser* self, FILE* stream) } } -int option_parser_getopt(struct option_parser* self, int argc, char* argv[]) +static const struct wv_option* find_long_option( + const struct option_parser* self, const char* name) { - return getopt_long(argc, argv, self->short_opts, self->long_opts, NULL); + for (int i = 0; i < self->n_opts; ++i) { + if (!self->options[i].long_opt) + continue; + + if (strcmp(self->options[i].long_opt, name) == 0) + return &self->options[i]; + } + return NULL; +} + +static const struct wv_option* find_short_option( + const struct option_parser* self, char name) +{ + for (int i = 0; i < self->n_opts; ++i) + if (self->options[i].short_opt == name) + return &self->options[i]; + return NULL; +} + +static const struct wv_option* find_positional_option( + struct option_parser* self, int position) +{ + int current_pos = 0; + for (int i = 0; i < self->n_opts; ++i) { + if (!self->options[i].positional) + continue; + + if (current_pos == position) + return &self->options[i]; + + current_pos += 1; + } + return NULL; +} + +static int append_value(struct option_parser* self, + const struct wv_option* option, const char* value) +{ + if ((size_t)self->n_values >= ARRAY_SIZE(self->values)) { + fprintf(stderr, "ERROR: Too many arguments!\n"); + return -1; + } + + struct wv_option_value* dst = &self->values[self->n_values++]; + dst->option = option; + strlcpy(dst->value, value, sizeof(dst->value)); + + return 0; +} + +static int parse_long_arg(struct option_parser* self, int argc, + const char* const* argv, int index) +{ + int count = 1; + char name[256]; + strlcpy(name, argv[index] + 2, sizeof(name)); + char* eq = strchr(name, '='); + if (eq) + *eq = '\0'; + + const struct wv_option* opt = find_long_option(self, name); + if (!opt) { + fprintf(stderr, "ERROR: Unknown option: \"%s\"\n", name); + return -1; + } + + const char* value = "1"; + if (opt->schema) { + if (eq) { + value = eq + 1; + } else { + if (index + 1 >= argc) { + fprintf(stderr, "ERROR: An argument is required for the \"%s\" option\n", + opt->long_opt); + return -1; + } + + value = argv[index + 1]; + count += 1; + } + } + + if (append_value(self, opt, value) < 0) + return -1; + + return count; +} + +static int parse_short_args(struct option_parser* self, char argc, + const char* const* argv, int index) +{ + int count = 1; + int len = strlen(argv[index]); + + for (int i = 1; i < len; ++i) { + char name = argv[index][i]; + const struct wv_option* opt = find_short_option(self, name); + if (!opt) { + fprintf(stderr, "ERROR: Unknown option: \"%c\"\n", name); + return -1; + } + + const char* value = "1"; + if (opt->schema) { + const char* tail = argv[index] + i + 1; + if (tail[0] == '=') { + value = tail + 1; + } else if (tail[0]) { + value = tail; + } else { + if (index + 1 >= argc) { + fprintf(stderr, "ERROR: An argument is required for the \"%c\" option\n", + opt->short_opt); + + return -1; + } + + value = argv[index + 1]; + count += 1; + } + } + + if (append_value(self, opt, value) < 0) + return -1; + + if (opt->schema) + break; + } + + return count; +} + +static int parse_positional_arg(struct option_parser* self, char argc, + const char* const* argv, int i) +{ + const struct wv_option* opt = find_positional_option(self, self->position); + if (!opt) + return 1; + + if (append_value(self, opt, argv[i]) < 0) + return -1; + + self->position += 1; + + return opt->is_subcommand ? 0 : 1; +} + +int option_parser_parse(struct option_parser* self, int argc, + const char* const* argv) +{ + int i = 1; + while (i < argc) { + if (argv[i][0] == '-') { + if (argv[i][1] == '-') { + if (argv[i][2] == '\0') + return 0; + + int rc = parse_long_arg(self, argc, argv, i); + if (rc < 0) + return -1; + i += rc; + } else { + int rc = parse_short_args(self, argc, argv, i); + if (rc < 0) + return -1; + i += rc; + } + } else { + int rc = parse_positional_arg(self, argc, argv, i); + if (rc < 0) + return -1; + if (rc == 0) + break; + i += rc; + } + } + self->endpos = i; + return 0; +} + +const char* option_parser_get_value(const struct option_parser* self, + const char* name) +{ + const struct wv_option* opt; + + bool is_short = name[0] && !name[1]; + + for (int i = 0; i < self->n_values; ++i) { + const struct wv_option_value* value = &self->values[i]; + opt = value->option; + + if (is_short) { + if (opt->short_opt && opt->short_opt == *name) + return value->value; + } else { + if (opt->long_opt && strcmp(opt->long_opt, name) == 0) + return value->value; + } + + if (opt->positional && strcmp(opt->positional, name) == 0) + return value->value; + } + + if (is_short) { + opt = find_short_option(self, name[0]); + if (opt) + return opt->default_; + } else { + opt = find_long_option(self, name); + if (opt) + return opt->default_; + } + + // TODO: Add positional option? + + return NULL; } diff --git a/src/wayvncctl.c b/src/wayvncctl.c index b3a7cc9..4170705 100644 --- a/src/wayvncctl.c +++ b/src/wayvncctl.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -71,6 +70,8 @@ int main(int argc, char* argv[]) unsigned flags = 0; static const struct wv_option opts[] = { + { .positional = "command", + .is_subcommand = true }, { 'S', "socket", "", "Control socket path." }, { 'w', "wait", NULL, @@ -85,46 +86,36 @@ int main(int argc, char* argv[]) "Be more verbose." }, { 'h', "help", NULL, "Get help (this text)." }, - { '\0', NULL, NULL, NULL } + { } }; struct option_parser option_parser; - option_parser_init(&option_parser, opts, - OPTION_PARSER_STOP_ON_FIRST_NONOPTION); - - while (1) { - int c = option_parser_getopt(&option_parser, argc, argv); - if (c < 0) - break; - - switch (c) { - case 'S': - socket_path = optarg; - break; - case 'w': - flags |= CTL_CLIENT_SOCKET_WAIT; - break; - case 'r': - flags |= CTL_CLIENT_RECONNECT; - break; - case 'j': - flags |= CTL_CLIENT_PRINT_JSON; - break; - case 'v': - verbose = true; - break; - case 'V': - return show_version(); - case 'h': - return wayvncctl_usage(stdout, &option_parser, 0); - default: - return wayvncctl_usage(stderr, &option_parser, 1); - } - } - argc -= optind; - if (argc <= 0) + option_parser_init(&option_parser, opts); + if (option_parser_parse(&option_parser, argc, + (const char* const*)argv) < 0) return wayvncctl_usage(stderr, &option_parser, 1); - argv = &argv[optind]; + + if (option_parser_get_value(&option_parser, "help")) + return wayvncctl_usage(stdout, &option_parser, 0); + + if (option_parser_get_value(&option_parser, "version")) + return show_version(); + + socket_path = option_parser_get_value(&option_parser, "socket"); + flags |= option_parser_get_value(&option_parser, "wait") + ? CTL_CLIENT_SOCKET_WAIT : 0; + flags |= option_parser_get_value(&option_parser, "reconnect") + ? CTL_CLIENT_RECONNECT : 0; + flags |= option_parser_get_value(&option_parser, "json") + ? CTL_CLIENT_PRINT_JSON : 0; + verbose = !!option_parser_get_value(&option_parser, "verbose"); + + // No command; nothing to do... + if (!option_parser_get_value(&option_parser, "command")) + return 0; + + argc -= option_parser.endpos; + argv += option_parser.endpos; ctl_client_debug_log(verbose); diff --git a/test/option-parser-test.c b/test/option-parser-test.c new file mode 100644 index 0000000..fe0286f --- /dev/null +++ b/test/option-parser-test.c @@ -0,0 +1,214 @@ +#include "tst.h" + +#include "option-parser.h" + +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) + +static const struct wv_option options[] = { + { .positional = "first" }, + { .positional = "second" }, + { .positional = "third" }, + { .positional = "command", .is_subcommand = true }, + { 'a', "option-a", NULL, "Description of a" }, + { 'b', "option-b", NULL, "Description of b" }, + { 'v', "value-option", "value", "Description of v" }, + { }, +}; + +static int test_simple(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + + const char* argv[] = { + "executable", + "-a", + "-b", + "pos 1", + "pos 2", + }; + + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_STR_EQ("pos 1", option_parser_get_value(&parser, "first")); + ASSERT_STR_EQ("pos 2", option_parser_get_value(&parser, "second")); + ASSERT_FALSE(option_parser_get_value(&parser, "third")); + ASSERT_TRUE(option_parser_get_value(&parser, "a")); + ASSERT_TRUE(option_parser_get_value(&parser, "option-b")); + ASSERT_FALSE(option_parser_get_value(&parser, "value-option")); + + return 0; +} + +static int test_short_value_option_with_space(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-v", "value" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_STR_EQ("value", option_parser_get_value(&parser, "value-option")); + return 0; +} + +static int test_short_value_option_without_space(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-vvalue" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_STR_EQ("value", option_parser_get_value(&parser, "value-option")); + return 0; +} + +static int test_short_value_option_with_eq(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-v=value" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_STR_EQ("value", option_parser_get_value(&parser, "value-option")); + return 0; +} + +static int test_long_value_option_with_space(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "--value-option", "value" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_STR_EQ("value", option_parser_get_value(&parser, "value-option")); + return 0; +} + +static int test_long_value_option_without_space(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "--value-option=value" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_STR_EQ("value", option_parser_get_value(&parser, "value-option")); + return 0; +} + +static int test_multi_short_option(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-ab" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_TRUE(option_parser_get_value(&parser, "a")); + ASSERT_TRUE(option_parser_get_value(&parser, "b")); + return 0; +} + +static int test_multi_short_option_with_value(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-abvthe-value" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_TRUE(option_parser_get_value(&parser, "a")); + ASSERT_TRUE(option_parser_get_value(&parser, "b")); + ASSERT_STR_EQ("the-value", option_parser_get_value(&parser, "v")); + return 0; +} + +static int test_stop(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "exec", "-a", "--", "-b"}; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + + ASSERT_TRUE(option_parser_get_value(&parser, "a")); + ASSERT_FALSE(option_parser_get_value(&parser, "b")); + return 0; +} + +static int test_unknown_short_option(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-x" }; + ASSERT_INT_EQ(-1, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + return 0; +} + +static int test_unknown_long_option(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "--an-unknown-long-option" }; + ASSERT_INT_EQ(-1, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + return 0; +} + +static int test_missing_short_value(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-v" }; + ASSERT_INT_EQ(-1, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + return 0; +} + +static int test_missing_long_value(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "--value-option" }; + ASSERT_INT_EQ(-1, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + return 0; +} + +static int test_subcommand_without_arguments(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-ab", "first", "second", "third", + "do-stuff" }; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + ASSERT_INT_EQ(5, parser.endpos); + ASSERT_STR_EQ("do-stuff", option_parser_get_value(&parser, "command")); + return 0; +} + +static int test_subcommand_with_arguments(void) +{ + struct option_parser parser; + option_parser_init(&parser, options); + const char* argv[] = { "executable", "-ab", "first", "second", "third", + "do-stuff", "--some-option", "another-argument"}; + ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv)); + ASSERT_INT_EQ(5, parser.endpos); + ASSERT_STR_EQ("do-stuff", option_parser_get_value(&parser, "command")); + return 0; +} + +int main() +{ + int r = 0; + RUN_TEST(test_simple); + RUN_TEST(test_short_value_option_with_space); + RUN_TEST(test_short_value_option_without_space); + RUN_TEST(test_short_value_option_with_eq); + RUN_TEST(test_long_value_option_with_space); + RUN_TEST(test_long_value_option_without_space); + RUN_TEST(test_multi_short_option); + RUN_TEST(test_multi_short_option_with_value); + RUN_TEST(test_stop); + RUN_TEST(test_unknown_short_option); + RUN_TEST(test_unknown_long_option); + RUN_TEST(test_missing_short_value); + RUN_TEST(test_missing_long_value); + RUN_TEST(test_subcommand_without_arguments); + RUN_TEST(test_subcommand_with_arguments); + return r; +}