From fdfc8536c1ef29ecaac8235913057547353e9266 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 22 Oct 2024 12:21:09 -0700 Subject: [PATCH 1/4] Add --named options --- picoquic/config.c | 123 ++++++++++++++++++++++++++++--------- picoquic/picoquic_config.h | 8 +++ picoquictest/config_test.c | 80 +++++++++++++++++++++++- 3 files changed, 181 insertions(+), 30 deletions(-) diff --git a/picoquic/config.c b/picoquic/config.c index 9c0505131..0f2cdba46 100644 --- a/picoquic/config.c +++ b/picoquic/config.c @@ -579,46 +579,75 @@ int picoquic_config_set_option(picoquic_quic_config_t* config, picoquic_option_e return ret; } -int picoquic_config_command_line(int opt, int * p_optind, int argc, char const ** argv, char const* optarg, picoquic_quic_config_t * config) +int picoquic_config_get_option_char_index(int opt) { - int ret = 0; int option_index = -1; - option_param_t params[5]; - int nb_params = 0; - /* Get the parameters */ for (size_t i = 0; i < option_table_size; i++) { if (option_table[i].option_letter == opt) { option_index = (int)i; break; } } + return option_index; +} - if (option_index == -1) { - fprintf(stderr, "Unknown option: -%c\n", opt); +int picoquic_config_get_option_name_index(char const * s, size_t l) +{ + int option_index = -1; + + for (size_t i = 0; i < option_table_size; i++) { + if (strncmp(s, option_table[i].option_name, l) == 0) { + option_index = (int)i; + break; + } } - else { - if (option_table[option_index].nb_params_required > 0) { - params[0].param = optarg; - if (optarg == NULL) { - fprintf(stderr, "option -%c requires %d arguments\n", opt, option_table[option_index].nb_params_required); - ret = -1; - } - else { - params[0].length = strlen(optarg); - nb_params++; - while (nb_params < option_table[option_index].nb_params_required) { - if (*p_optind + 1 > argc) { - fprintf(stderr, "option -%c requires %d arguments\n", opt, option_table[option_index].nb_params_required); - ret = -1; - break; - } - else { - params[nb_params].param = argv[*p_optind]; - params[nb_params].length = (int)strlen(argv[*p_optind]); - nb_params++; - *p_optind += 1; - } + return option_index; +} + +int picoquic_config_get_command_line_option_index(char const* opt_string) +{ + int option_index = -1; + + if (opt_string[0] == '-' && opt_string[1] != 0) { + if (opt_string[2] == 0) { + option_index = picoquic_config_get_option_char_index(opt_string[1]); + } + else if (opt_string[1] == '-' && opt_string[2] != 0) { + char const* opt_name = opt_string + 2; + option_index = picoquic_config_get_option_name_index(opt_name, strlen(opt_name)); + } + } + return option_index; +} + +int picoquic_get_command_line_option_value(int option_index, char const * opt_string, int* p_optind, const char** argv, + int argc, char const* optarg, picoquic_quic_config_t* config) +{ + int ret = 0; + option_param_t params[5]; + int nb_params = 0; + + if (option_table[option_index].nb_params_required > 0) { + params[0].param = optarg; + if (optarg == NULL) { + fprintf(stderr, "option %s requires %d arguments\n", opt_string, option_table[option_index].nb_params_required); + ret = -1; + } + else { + params[0].length = strlen(optarg); + nb_params++; + while (nb_params < option_table[option_index].nb_params_required) { + if (*p_optind + 1 > argc) { + fprintf(stderr, "option %s requires %d arguments\n", opt_string, option_table[option_index].nb_params_required); + ret = -1; + break; + } + else { + params[nb_params].param = argv[*p_optind]; + params[nb_params].length = (int)strlen(argv[*p_optind]); + nb_params++; + *p_optind += 1; } } } @@ -631,6 +660,42 @@ int picoquic_config_command_line(int opt, int * p_optind, int argc, char const * return ret; } +int picoquic_config_command_line(int opt, int * p_optind, int argc, char const ** argv, char const* optarg, picoquic_quic_config_t * config) +{ + int ret = 0; + int option_index = -1; + char opt_string[3] = { '-', 0, 0 }; + + opt_string[1] = (char)opt; + option_index = picoquic_config_get_option_char_index(opt); + + if (option_index == -1) { + fprintf(stderr, "Unknown option: -%c\n", opt); + } + else { + ret = picoquic_get_command_line_option_value(option_index, opt_string, p_optind, + argv, argc, optarg, config); + } + return ret; +} + +int picoquic_config_command_line_ex(char const * opt_string, int* p_optind, int argc, char const** argv, char const* optarg, picoquic_quic_config_t* config) +{ + int ret = 0; + int option_index = -1; + + option_index = picoquic_config_get_command_line_option_index(opt_string); + + if (option_index == -1) { + fprintf(stderr, "Unknown option: %s\n", opt_string); + } + else { + ret = picoquic_get_command_line_option_value(option_index, opt_string, p_optind, + argv, argc, optarg, config); + } + return ret; +} + int picoquic_config_file(char const* file_name, picoquic_quic_config_t* config) { int ret = 0; diff --git a/picoquic/picoquic_config.h b/picoquic/picoquic_config.h index 74a16f2ee..4459da06f 100644 --- a/picoquic/picoquic_config.h +++ b/picoquic/picoquic_config.h @@ -130,7 +130,15 @@ int picoquic_config_option_letters(char* option_string, size_t string_max, size_ void picoquic_config_usage(); int picoquic_config_set_option(picoquic_quic_config_t* config, picoquic_option_enum_t option_num, const char* opt_val); +/* picoquic_config_command_line: +* parse options, with the restriction that all options must be identified by a single character, as in '-x' + */ int picoquic_config_command_line(int opt, int* p_optind, int argc, char const** argv, char const* optarg, picoquic_quic_config_t* config); +/* picoquic_config_command_line: +* parse options, allowing both single character options (e.g. -X) +* and named option (e.g. --disable_block) + */ +int picoquic_config_command_line_ex(char const* opt_string, int* p_optind, int argc, char const** argv, char const* optarg, picoquic_quic_config_t* config); int picoquic_config_file(char const* file_name, picoquic_quic_config_t* config); diff --git a/picoquictest/config_test.c b/picoquictest/config_test.c index b99daee28..7b3570f69 100644 --- a/picoquictest/config_test.c +++ b/picoquictest/config_test.c @@ -194,6 +194,25 @@ static const char* config_argv2[] = { NULL }; +static const char * config_two[] = { + "--sni", "test.example.com", + "--alpn", "test", + "--outdir", "/data/w_out", + "--root_trust_file", "data/certs/root.pem", + "--cipher_suite", "20", + "--proposed_version", "ff000020", + "--force_zero_share", + "--idle_timeout", "1234567", + "--no_disk", + "--large_client_hello", + "--disable_block", + "--cnxid_length", "5", + "--ticket_file", "/data/tickets.bin", + "--token_file", "/data/tokens.bin", + "--version_upgrade", "00000002", + NULL +}; + int config_test_compare_string(const char* title, const char* expected, const char* actual) { int ret = 0; @@ -335,18 +354,77 @@ int config_test_parse_command_line(const picoquic_quic_config_t* expected, const return (ret); } + +int config_test_parse_command_line_ex(const picoquic_quic_config_t* expected, const char** argv, int argc) +{ + int ret = 0; + int opt_ind = 0; + picoquic_quic_config_t actual; + + picoquic_config_init(&actual); + + while (opt_ind < argc && ret == 0) { + const char* x = argv[opt_ind]; + const char* optval = NULL; + + if (x == NULL) { + /* could not parse to the end! */ + DBG_PRINTF("Unexpected stop after %d arguments, expected %d", opt_ind, argc); + ret = -1; + break; + } + else if (x[0] != '-' || x[1] == 0 || + (x[2] != 0 && x[1] != '-')) { + /* could not parse to the end! */ + DBG_PRINTF("Unexpected argument: %s", x); + ret = -1; + break; + } + opt_ind++; + if (opt_ind < argc) { + optval = argv[opt_ind]; + if (optval[0] == '-') { + optval = NULL; + } + else { + opt_ind++; + } + } + ret = picoquic_config_command_line_ex(x, &opt_ind, argc, argv, optval, &actual); + if (ret != 0) { + DBG_PRINTF("Could not parse opt %s", x); + } + } + + if (ret == 0) { + ret = config_test_compare(expected, &actual); + } + + picoquic_config_clear(&actual); + + return (ret); +} + + int config_option_test() { int ret = config_test_parse_command_line(¶m1, config_argv1, (int)(sizeof(config_argv1) / sizeof(char const*)) - 1); if (ret != 0) { DBG_PRINTF("First config option test returns %d", ret); } - else { + if (ret == 0) { ret = config_test_parse_command_line(¶m2, config_argv2, (int)(sizeof(config_argv2) / sizeof(char const*)) - 1); if (ret != 0) { DBG_PRINTF("Second config option test returns %d", ret); } } + if (ret == 0) { + ret = config_test_parse_command_line_ex(¶m2, config_two, (int)(sizeof(config_two) / sizeof(char const*)) - 1); + if (ret != 0) { + DBG_PRINTF("Two dash config option test returns %d", ret); + } + } + return ret; } From 2403ee8bd1263da138b4e5858ed4663cad8f56bc Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 22 Oct 2024 12:24:46 -0700 Subject: [PATCH 2/4] Restore state of file options. --- picoquic/config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/picoquic/config.c b/picoquic/config.c index 0f2cdba46..09fd0aeaa 100644 --- a/picoquic/config.c +++ b/picoquic/config.c @@ -696,6 +696,10 @@ int picoquic_config_command_line_ex(char const * opt_string, int* p_optind, int return ret; } +#if 0 +/* Reading parameters from a file instead of the command line. +* Apparently never used, also not tested. + */ int picoquic_config_file(char const* file_name, picoquic_quic_config_t* config) { int ret = 0; @@ -769,7 +773,7 @@ int picoquic_config_file(char const* file_name, picoquic_quic_config_t* config) return ret; } - +#endif /* Create a QUIC Context based on configuration data. From 243476419b6108bd7db18153faeabe84bef463c6 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 22 Oct 2024 13:44:43 -0700 Subject: [PATCH 3/4] Fix merge issues --- picoquictest/config_test.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/picoquictest/config_test.c b/picoquictest/config_test.c index 35d0edffd..2aa773216 100644 --- a/picoquictest/config_test.c +++ b/picoquictest/config_test.c @@ -22,6 +22,7 @@ #include #include +#include "picoquic.h" #include "picoquic_internal.h" #include "picoquic_utils.h" #include "picoquic_config.h" @@ -535,7 +536,7 @@ int config_option_test() DBG_PRINTF("First config option test returns %d", ret); } if (ret == 0) { - ret = config_test_parse_command_line(¶m2, config_argv2, (int)(sizeof(config_argv2) / sizeof(char const*)) - 1); + ret = config_parse_command_line_test(¶m2, config_argv2, (int)(sizeof(config_argv2) / sizeof(char const*)) - 1); if (ret != 0) { DBG_PRINTF("Second config option test returns %d", ret); @@ -546,14 +547,15 @@ int config_option_test() ret = config_test_parse_command_line_ex(¶m2, config_two, (int)(sizeof(config_two) / sizeof(char const*)) - 1); if (ret != 0) { DBG_PRINTF("Two dash config option test returns %d", ret); - } + } - for (size_t i = 0; ret == 0 && i < nb_config_errors; i++) { - picoquic_quic_config_t config = { 0 }; - if (config_parse_command_line(&config, config_errors[i].err_args, - config_errors[i].nb_args, 1) == 0) { - DBG_PRINTF("Did not detect config error %zu, %s", i, config_errors[i].err_args[0]); - ret = -1; + for (size_t i = 0; ret == 0 && i < nb_config_errors; i++) { + picoquic_quic_config_t config = { 0 }; + if (config_parse_command_line(&config, config_errors[i].err_args, + config_errors[i].nb_args, 1) == 0) { + DBG_PRINTF("Did not detect config error %zu, %s", i, config_errors[i].err_args[0]); + ret = -1; + } } } From 1c516ae6616af530488f1c6c5a37f6c1c55f3ebd Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 22 Oct 2024 18:09:50 -0700 Subject: [PATCH 4/4] Fix config test issues. --- picoquic/config.c | 1 + picoquictest/config_test.c | 18 +++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/picoquic/config.c b/picoquic/config.c index 7ab396da3..113d26ec1 100644 --- a/picoquic/config.c +++ b/picoquic/config.c @@ -609,6 +609,7 @@ int picoquic_config_command_line(int opt, int * p_optind, int argc, char const * if (option_index == -1) { fprintf(stderr, "Unknown option: -%c\n", opt); + ret = -1; } else { ret = picoquic_get_command_line_option_value(option_index, opt_string, p_optind, diff --git a/picoquictest/config_test.c b/picoquictest/config_test.c index 2aa773216..c75214545 100644 --- a/picoquictest/config_test.c +++ b/picoquictest/config_test.c @@ -229,10 +229,14 @@ static const char * config_two[] = { "--no_disk", "--large_client_hello", "--disable_block", +#ifndef PICOQUIC_WITHOUT_SSLKEYLOG + "--sslkeylog", +#endif "--cnxid_length", "5", "--ticket_file", "/data/tickets.bin", "--token_file", "/data/tokens.bin", "--version_upgrade", "00000002", + "--cwin_max", "1000000", NULL }; @@ -548,14 +552,14 @@ int config_option_test() if (ret != 0) { DBG_PRINTF("Two dash config option test returns %d", ret); } + } - for (size_t i = 0; ret == 0 && i < nb_config_errors; i++) { - picoquic_quic_config_t config = { 0 }; - if (config_parse_command_line(&config, config_errors[i].err_args, - config_errors[i].nb_args, 1) == 0) { - DBG_PRINTF("Did not detect config error %zu, %s", i, config_errors[i].err_args[0]); - ret = -1; - } + for (size_t i = 0; ret == 0 && i < nb_config_errors; i++) { + picoquic_quic_config_t config = { 0 }; + if (config_parse_command_line(&config, config_errors[i].err_args, + config_errors[i].nb_args, 1) == 0) { + DBG_PRINTF("Did not detect config error %zu, %s", i, config_errors[i].err_args[0]); + ret = -1; } }