This fixes a bug with --config=<filename> handling.
Add expanded test code courtesy of Frediano Ziglio <fziglio@xxxxxxxxxx>.
Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx>
---
src/main.c | 8 +++--
src/options.c | 93 ++++++++++++++++++++++++++++++++++++++++++---------
src/options.h | 1 -
3 files changed, 83 insertions(+), 19 deletions(-)
diff --git a/src/main.c b/src/main.c
index f18311c9..da6d4882 100644
--- a/src/main.c
+++ b/src/main.c
@@ -67,9 +67,13 @@ 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);
+ if (rc == 0) {
+ options_from_config(&session.options);
+ /* We parse command line arguments a second time to ensure
+ ** that command line options take precedence over config files */
+ rc = options_parse_arguments(argc, argv, &session.options);
+ }
if (rc)
goto exit;
diff --git a/src/options.c b/src/options.c
index bc7cd631..6620b853 100644
--- a/src/options.c
+++ b/src/options.c
@@ -214,16 +214,6 @@ void options_handle_ssl_file_options(options_t *options,
string_option(&options->ssl.ciphersuite, 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) {
- str_replace(&options->user_config_file, argv[i + 1]);
- i++;
- }
-}
-
int options_parse_arguments(int argc, char *argv[], options_t *options)
{
int rc;
@@ -254,6 +244,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) {
@@ -287,7 +278,7 @@ int options_parse_arguments(int argc, char *argv[], options_t *options)
break;
case OPTION_CONFIG:
- /* This was handled previously; we can ignore */
+ str_replace(&options->user_config_file, optarg);
break;
case OPTION_SSL:
@@ -501,15 +492,85 @@ int options_impossible_config(options_t *options)
return 1;
}
-#if defined(OPTIONS_MAIN)
-int main(int argc, char *argv[])
+#if defined(MAIN_TEST)
+static int test_load_options(options_t *options, int argc, char **argv)
+{
+ int rc;
+ rc = options_parse_arguments(argc, argv, options);
+ if (rc == 0) {
+ options_from_config(options);
+ /* We parse command line arguments a second time to ensure
+ ** that command line options take precedence over config files */
+ rc = options_parse_arguments(argc, argv, options);
+ }
+ if (rc == 0) {
+ rc = options_process_io(options);
+ }
+ return rc;
+}
+
+static void test(const char *expected, char **argv)
{
options_t options;
+ int argc = 0;
+ int rc;
+ char buf[1000];
+
+ while (argv[argc]) {
+ ++argc;
+ }
options_init(&options);
- options_parse_arguments(argc, argv, &options);
- options_from_config(&options);
- g_message("Options parsed");
+ rc = test_load_options(&options, argc, argv);
+ if (rc == 0) {
+ snprintf(buf, sizeof(buf), "display %s allow_control %d listen %s ca_file %s",
+ options.display, options.allow_control, options.listen, options.ssl.ca_cert_file);
+ if (strcmp(buf, expected) != 0) {
+ fprintf(stderr, "Unexpected results:\n\t%s\n%s\n",
+ buf, expected);
+ }
+ }
+ else {
+ fprintf(stderr, "Unable to load options, rc %d\n", rc);
+ }
options_free(&options);
}
+
+#define TEST(exp, ...) do { \
+ char *argv[] = { "PROGRAM", __VA_ARGS__, NULL }; \
+ test(exp, argv); \
+} while(0)
+
+int main(int argc, char **argv)
+{
+ FILE *f = fopen("test.conf", "w");
+
+ if (argc > 1) {
+ options_t options;
+ options_init(&options);
+ printf("test_load_options returns %d\n",
+ test_load_options(&options, argc, argv));
+ options_free(&options);
+ }
+
+ fprintf(f, "[spice]\ndisplay=config_display\nlisten=8765\nallow-control=true");
+ fclose(f);
+
+ TEST("display (null) allow_control 0 listen 5900 ca_file (null)",
+ "--hide");
+ TEST("display (null) allow_control 0 listen 1234 ca_file (null)",
+ "1234");
+ TEST("display config_display allow_control 1 listen 8765 ca_file (null)",
+ "--config=test.conf");
+ TEST("display config_display allow_control 1 listen 123 ca_file (null)",
+ "--config=test.conf", "123");
+ TEST("display config_display allow_control 0 listen 123 ca_file (null)",
+ "--no-allow-control", "--config=test.conf", "123");
+ TEST("display DISPLAY allow_control 1 listen 123 ca_file (null)",
+ "--display", "DISPLAY", "--config=test.conf", "123");
+ TEST("display (null) allow_control 0 listen 5900 ca_file foo",
+ "--ssl=foo");
+ unlink("test.conf");
+ return 0;
+}
#endif
diff --git a/src/options.h b/src/options.h
index 6155984b..2302c85b 100644
--- a/src/options.h
+++ b/src/options.h
@@ -73,7 +73,6 @@ typedef struct {
** Prototypes
**--------------------------------------------------------------------------*/
void options_init(options_t *options);
-void 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);
--
2.20.1
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel