[Spice-devel] [PATCH v4 x11spice 1/3] Convert to the use of glib memory routines in options.c.

Jeremy White jwhite at codeweavers.com
Tue Jul 23 21:28:56 UTC 2019


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 at codeweavers.com>
---
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



More information about the Spice-devel mailing list