Also be sure that when replacing an option string, we free it first, and free the SSL and password_file option fields. Signed-off-by: Jeremy White <jwhite@xxxxxxxxxxxxxxx> --- v4: Consolidate a few patches into one Use a suggestion from Freddy to free memory on replace Use that to simplify the expression of argument procesing further Bring in the test code written by Freddy src/options.c | 125 +++++++++++++++++++++++++------------------------- 1 file changed, 62 insertions(+), 63 deletions(-) diff --git a/src/options.c b/src/options.c index b7f487c5..bc7cd631 100644 --- a/src/options.c +++ b/src/options.c @@ -45,43 +45,46 @@ #include <libaudit.h> #endif +static void str_replace(char **dest, const char *src) +{ + g_free(*dest); + *dest = src ? g_strdup(src) : NULL; +} + void options_init(options_t *options) { memset(options, 0, sizeof(*options)); } -void options_free(options_t *options) +static void ssl_options_free(ssl_options_t *ssl) { - if (options->display) { - free(options->display); - options->display = NULL; - } - - g_free(options->spice_password); - options->spice_password = NULL; - - g_free(options->virtio_path); - options->virtio_path = NULL; - g_free(options->uinput_path); - options->uinput_path = NULL; - g_free(options->on_connect); - options->on_connect = NULL; - g_free(options->on_disconnect); - options->on_disconnect = NULL; - - if (options->listen) - free(options->listen); - options->listen = NULL; - - g_free(options->user_config_file); - options->user_config_file = NULL; + str_replace(&ssl->ca_cert_file, NULL); + str_replace(&ssl->certs_file, NULL); + str_replace(&ssl->private_key_file, NULL); + str_replace(&ssl->key_password, NULL); + str_replace(&ssl->dh_key_file, NULL); + str_replace(&ssl->ciphersuite, NULL); +} - g_free(options->system_config_file); - options->system_config_file = NULL; +void options_free(options_t *options) +{ + str_replace(&options->display, NULL); + str_replace(&options->listen, NULL); + + ssl_options_free(&options->ssl); + str_replace(&options->spice_password, NULL); + str_replace(&options->password_file, NULL); + + str_replace(&options->virtio_path, NULL); + str_replace(&options->uinput_path, NULL); + str_replace(&options->on_connect, NULL); + str_replace(&options->on_disconnect, NULL); + str_replace(&options->user_config_file, NULL); + str_replace(&options->system_config_file, NULL); } -static gchar *string_option(GKeyFile *u, GKeyFile *s, const gchar *section, const gchar *key) +static void string_option(gchar **dest, GKeyFile *u, GKeyFile *s, const gchar *section, const gchar *key) { gchar *ret = NULL; GError *error = NULL; @@ -93,7 +96,8 @@ static gchar *string_option(GKeyFile *u, GKeyFile *s, const gchar *section, cons if (error) g_error_free(error); - return ret; + g_free(*dest); + *dest = ret; } static gint int_option(GKeyFile *u, GKeyFile *s, const gchar *section, const gchar *key) @@ -159,36 +163,33 @@ static void usage(char *argv0) int options_handle_ssl(options_t *options, const char *spec) { char *save = NULL; - char *in = strdup(spec); + char *in = g_strdup(spec); char *p; int i = 0; int rc = 0; - if (!in) - return X11SPICE_ERR_MALLOC; - for (p = strtok_r(in, ",", &save); p; p = strtok_r(NULL, ",", &save), i++) { if (strlen(p) == 0) continue; switch(i) { case 0: - options->ssl.ca_cert_file = strdup(p); + str_replace(&options->ssl.ca_cert_file, p); break; case 1: - options->ssl.certs_file = strdup(p); + str_replace(&options->ssl.certs_file, p); break; case 2: - options->ssl.private_key_file = strdup(p); + str_replace(&options->ssl.private_key_file, p); break; case 3: - options->ssl.key_password = strdup(p); + str_replace(&options->ssl.key_password, p); break; case 4: - options->ssl.dh_key_file = strdup(p); + str_replace(&options->ssl.dh_key_file, p); break; case 5: - options->ssl.ciphersuite = strdup(p); + str_replace(&options->ssl.ciphersuite, p); break; default: fprintf(stderr, "Error: invalid ssl specification."); @@ -197,7 +198,7 @@ int options_handle_ssl(options_t *options, const char *spec) } } - free(in); + g_free(in); return rc; } @@ -205,12 +206,12 @@ void options_handle_ssl_file_options(options_t *options, GKeyFile *userkey, GKeyFile *systemkey) { options->ssl.enabled = bool_option(userkey, systemkey, "ssl", "enabled"); - options->ssl.ca_cert_file = string_option(userkey, systemkey, "ssl", "ca-cert-file"); - options->ssl.certs_file = string_option(userkey, systemkey, "ssl", "certs-file"); - options->ssl.private_key_file = string_option(userkey, systemkey, "ssl", "private-key-file"); - options->ssl.key_password = string_option(userkey, systemkey, "ssl", "key-password-file"); - options->ssl.dh_key_file = string_option(userkey, systemkey, "ssl", "dh-key-file"); - options->ssl.ciphersuite = string_option(userkey, systemkey, "ssl", "ciphersuite"); + string_option(&options->ssl.ca_cert_file, userkey, systemkey, "ssl", "ca-cert-file"); + string_option(&options->ssl.certs_file, userkey, systemkey, "ssl", "certs-file"); + string_option(&options->ssl.private_key_file, userkey, systemkey, "ssl", "private-key-file"); + string_option(&options->ssl.key_password, userkey, systemkey, "ssl", "key-password-file"); + string_option(&options->ssl.dh_key_file, userkey, systemkey, "ssl", "dh-key-file"); + string_option(&options->ssl.ciphersuite, userkey, systemkey, "ssl", "ciphersuite"); } void options_handle_user_config(int argc, char *argv[], options_t *options) @@ -218,7 +219,7 @@ 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 = strdup(argv[i + 1]); + str_replace(&options->user_config_file, argv[i + 1]); i++; } } @@ -278,11 +279,11 @@ int options_parse_arguments(int argc, char *argv[], options_t *options) break; case OPTION_PASSWORD: - options->spice_password = strdup(optarg); + str_replace(&options->spice_password, optarg); break; case OPTION_PASSWORD_FILE: - options->password_file = strdup(optarg); + str_replace(&options->password_file, optarg); break; case OPTION_CONFIG: @@ -305,7 +306,7 @@ int options_parse_arguments(int argc, char *argv[], options_t *options) break; case OPTION_DISPLAY: - options->display = strdup(optarg); + str_replace(&options->display, optarg); break; case OPTION_MINIMIZE: @@ -335,12 +336,12 @@ int options_parse_arguments(int argc, char *argv[], options_t *options) if (rc == 0) { if (optind >= argc) { /* Default */ - options->listen = strdup("5900"); + str_replace(&options->listen, "5900"); } else if (optind < (argc - 1)) { fprintf(stderr, "Error: too many arguments\n"); rc = X11SPICE_ERR_BADARGS; } else { - options->listen = strdup(argv[optind]); + str_replace(&options->listen, argv[optind]); } } @@ -375,17 +376,17 @@ void options_from_config(options_t *options) options->allow_control = bool_option(userkey, systemkey, "spice", "allow-control"); options->generate_password = int_option(userkey, systemkey, "spice", "generate-password"); options->hide = bool_option(userkey, systemkey, "spice", "hide"); - options->display = string_option(userkey, systemkey, "spice", "display"); + string_option(&options->display, userkey, systemkey, "spice", "display"); - options->listen = string_option(userkey, systemkey, "spice", "listen"); - options->spice_password = string_option(userkey, systemkey, "spice", "password"); - options->password_file = string_option(userkey, systemkey, "spice", "password-file"); + string_option(&options->listen, userkey, systemkey, "spice", "listen"); + string_option(&options->spice_password, userkey, systemkey, "spice", "password"); + string_option(&options->password_file, userkey, systemkey, "spice", "password-file"); options->disable_ticketing = bool_option(userkey, systemkey, "spice", "disable-ticketing"); options->exit_on_disconnect = bool_option(userkey, systemkey, "spice", "exit-on-disconnect"); - options->virtio_path = string_option(userkey, systemkey, "spice", "virtio-path"); - options->uinput_path = string_option(userkey, systemkey, "spice", "uinput-path"); - options->on_connect = string_option(userkey, systemkey, "spice", "on-connect"); - options->on_disconnect = string_option(userkey, systemkey, "spice", "on-disconnect"); + string_option(&options->virtio_path, userkey, systemkey, "spice", "virtio-path"); + string_option(&options->uinput_path, userkey, systemkey, "spice", "uinput-path"); + string_option(&options->on_connect, userkey, systemkey, "spice", "on-connect"); + string_option(&options->on_disconnect, userkey, systemkey, "spice", "on-disconnect"); options->audit = bool_option(userkey, systemkey, "spice", "audit"); options->audit_message_type = int_option(userkey, systemkey, "spice", "audit-message-type"); @@ -434,7 +435,7 @@ static int process_password_file(options_t *options) if (p > buf && *(p - 1) == '\n') *(p - 1) = '\0'; - options->spice_password = strdup(buf); + str_replace(&options->spice_password, buf); return rc; } @@ -449,9 +450,7 @@ static int generate_password(options_t *options) if (fd < 0) return X11SPICE_ERR_OPEN; - p = options->spice_password = malloc(options->generate_password + 1); - if (!p) - return X11SPICE_ERR_MALLOC; + p = options->spice_password = g_malloc(options->generate_password + 1); while (p - options->spice_password < options->generate_password) { rc = read(fd, p, sizeof(*p)); -- 2.20.1 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel