> > Instead of trying to parse argv ourselves, just reuse getopt_long_only. > > Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx> This patch looks a bit long compared to the job is doing. What is the expected behaviour mixing --config and other options? It looks like the configuration file should be always read first, all other options, before or after --config should override the configuration file. Is it right? Why don't you call options_parse_arguments and if you find the configuration file you then call options_handle_user_config and options_parse_arguments again to just override the options loaded from the file? Was testing something like https://cgit.freedesktop.org/~fziglio/x11spice/commit/?h=options&id=3595c4f06c0df5056e4cb65367a1de72b0c66056 with https://cgit.freedesktop.org/~fziglio/x11spice/commit/?h=options&id=ef824f30a1a91ee77f44be7d86861535f7323b0a (on top of your patches) > --- > src/main.c | 8 +++++--- > src/options.c | 50 ++++++++++++++++++++++++++++++++++++-------------- > src/options.h | 2 +- > 3 files changed, 42 insertions(+), 18 deletions(-) > > diff --git a/src/main.c b/src/main.c > index f18311c9..72aba5a8 100644 > --- a/src/main.c > +++ b/src/main.c > @@ -67,9 +67,11 @@ int main(int argc, char *argv[]) > ** Parse arguments > **----------------------------------------------------------------------*/ > options_init(&session.options); > - options_handle_user_config(argc, argv, &session.options); > - options_from_config(&session.options); > - rc = options_parse_arguments(argc, argv, &session.options); > + rc = options_handle_user_config(argc, argv, &session.options); > + if (rc == 0) { > + options_from_config(&session.options); > + rc = options_parse_arguments(argc, argv, &session.options); > + } > if (rc) > goto exit; > > diff --git a/src/options.c b/src/options.c > index 303c07de..5a55f2c9 100644 > --- a/src/options.c > +++ b/src/options.c > @@ -224,16 +224,6 @@ void options_handle_ssl_file_options(options_t *options, > options->ssl.ciphersuite = string_option(userkey, systemkey, "ssl", > "ciphersuite"); > } > > -void options_handle_user_config(int argc, char *argv[], options_t *options) > -{ > - int i; > - for (i = 1; i < argc - 1; i++) > - if (strcmp(argv[i], "--config") == 0 || strcmp(argv[i], "-config") > == 0) { > - options->user_config_file = g_strdup(argv[i + 1]); > - i++; > - } > -} > - > int options_parse_arguments(int argc, char *argv[], options_t *options) > { > int rc; > @@ -264,6 +254,7 @@ int options_parse_arguments(int argc, char *argv[], > options_t *options) > {0, 0, 0, 0} > }; > > + optind = 1; /* Allow reuse of this function */ > while (1) { > rc = getopt_long_only(argc, argv, "", long_options, &longindex); > if (rc == -1) { > @@ -297,7 +288,9 @@ int options_parse_arguments(int argc, char *argv[], > options_t *options) > break; > > case OPTION_CONFIG: > - /* This was handled previously; we can ignore */ > + if (options->user_config_file) > + g_free(options->user_config_file); > + options->user_config_file = g_strdup(optarg); > break; > > case OPTION_SSL: > @@ -358,6 +351,21 @@ int options_parse_arguments(int argc, char *argv[], > options_t *options) > return rc; > } > > +/* The idea is to have command line arguments run after all other > +** configuration, with one exception - we want to grab any > +** user specified configuration file first. */ > +int options_handle_user_config(int argc, char *argv[], options_t *options) > +{ > + options_t tmp_options = { 0 }; > + int rc; > + > + rc = options_parse_arguments(argc, argv, &tmp_options); > + options->user_config_file = tmp_options.user_config_file; > + tmp_options.user_config_file = NULL; > + options_free(&tmp_options); > + return rc; > +} > + > void options_from_config(options_t *options) > { > GKeyFile *userkey = g_key_file_new(); > @@ -517,11 +525,25 @@ int options_impossible_config(options_t *options) > int main(int argc, char *argv[]) > { > options_t options; > + int rc; > > options_init(&options); > - options_parse_arguments(argc, argv, &options); > - options_from_config(&options); > - g_message("Options parsed"); > + rc = options_handle_user_config(argc, argv, &options); > + if (rc == 0) { > + options_from_config(&options); > + rc = options_parse_arguments(argc, argv, &options); > + } > + if (rc == 0) { > + g_message("Options parsed"); > + rc = options_process_io(&options); > + if (rc == 0) > + g_message("Options io run parsed"); > + else > + g_message("Options io failed"); > + } > + else { > + g_message("Options parse failed"); > + } > options_free(&options); > } > #endif > diff --git a/src/options.h b/src/options.h > index 6155984b..162643ac 100644 > --- a/src/options.h > +++ b/src/options.h > @@ -73,7 +73,7 @@ typedef struct { > ** Prototypes > **--------------------------------------------------------------------------*/ > void options_init(options_t *options); > -void options_handle_user_config(int argc, char *argv[], options_t *options); > +int options_handle_user_config(int argc, char *argv[], options_t *options); > int options_parse_arguments(int argc, char *argv[], options_t *options); > int options_process_io(options_t *options); > void options_free(options_t *options); Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel