Fix memory bugs leading to leaks and crashes (#26)

cpdb/cpdb-frontend.c

@@ -940,7 +940,6 @@ cpdb_options_t *cpdbGetAllOptions(cpdb_printer_obj_t *p)
     if (p->options)
         return p->options;
 
-    p->options = cpdbGetNewOptions();
     GError *error = NULL;
     int num_options, num_media;
     GVariant *var, *media_var;
@@ -952,23 +951,18 @@ cpdb_options_t *cpdbGetAllOptions(cpdb_printer_obj_t *p)
                                             &media_var,
                                             NULL,
                                             &error);
-    if (!error)
-    {
-        loginfo("Obtained %d options and %d media for %s %s\n",
-                num_options, num_media, p->id, p->backend_name);
-        cpdbUnpackOptions(num_options,
-                          var,
-                          num_media,
-                          media_var,
-                          p->options);
-        return p->options;
-    }
-    else 
+    if (error)
     {
         logerror("Error getting printer options for %s %s : %s\n",
                     p->id, p->backend_name, error->message);
         return NULL;
     }
+
+    loginfo("Obtained %d options and %d media for %s %s\n",
+            num_options, num_media, p->id, p->backend_name);
+    p->options = cpdbGetNewOptions();
+    cpdbUnpackOptions(num_options, var, num_media, media_var, p->options);
+    return p->options;
 }
 
 cpdb_option_t *cpdbGetOption(cpdb_printer_obj_t *p,
@@ -1736,7 +1730,7 @@ cpdb_settings_t *cpdbGetNewSettings()
 {
     cpdb_settings_t *s = g_new0(cpdb_settings_t, 1);
     s->count = 0;
-    s->table = g_hash_table_new_full(g_str_hash, g_str_equal, free, free);
+    s->table = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
     return s;
 }
 
@@ -1767,24 +1761,11 @@ void cpdbAddSetting(cpdb_settings_t *s,
         return;
     }
 
-    char *prev = g_hash_table_lookup(s->table, name);
-    if (prev)
-    {
-        /**
-         * The value is already there, so replace it instead
-         */
-        g_hash_table_replace(s->table,
-                             cpdbGetStringCopy(name),
-                             cpdbGetStringCopy(val));
-        free(prev);
-    }
-    else
-    {
-        g_hash_table_insert(s->table,
-                            cpdbGetStringCopy(name),
-                            cpdbGetStringCopy(val));
+    gboolean new_entry = g_hash_table_insert(s->table,
+                                             cpdbGetStringCopy(name),
+                                             cpdbGetStringCopy(val));
+    if (new_entry)
         s->count++;
-    }
 }
 
 gboolean cpdbClearSetting(cpdb_settings_t *s, const char *name)
@@ -1935,12 +1916,12 @@ cpdb_options_t *cpdbGetNewOptions()
     o->count = 0;
     o->table = g_hash_table_new_full(g_str_hash,
                                      g_str_equal,
-                                     NULL,
+                                     g_free,
                                      (GDestroyNotify) cpdbDeleteOption);
     o->media_count = 0;
     o->media = g_hash_table_new_full(g_str_hash,
                                      g_str_equal,
-                                     NULL,
+                                     g_free,
                                      (GDestroyNotify) cpdbDeleteMedia);
     return o;
 }
@@ -2076,7 +2057,7 @@ void cpdbUnpackOptions(int num_options,
             logdebug("  %s;\n", str);
             opt->supported_values[j] = cpdbGetStringCopy(str);
         }
-        g_hash_table_insert(options->table, opt->option_name, opt);
+        g_hash_table_insert(options->table, cpdbGetStringCopy(opt->option_name), opt);
     }
 
     options->media_count = num_media;
@@ -2105,7 +2086,7 @@ void cpdbUnpackOptions(int num_options,
             media->margins[j].top = t; 
             media->margins[j].bottom = b;
 		}
-		g_hash_table_insert(options->media, media->name, media);
+		g_hash_table_insert(options->media, cpdbGetStringCopy(media->name), media);
 	}
 
 }

cpdb/cpdb.c

@@ -59,6 +59,11 @@ void cpdbInit()
     }
 }
 
+char **cpdbNewCStringArray(int num_elems)
+{
+    return malloc(sizeof(char *) * num_elems);
+}
+
 gboolean cpdbGetBoolean(const char *g)
 {
     if (!g)
@@ -140,14 +145,13 @@ GVariant *cpdbPackStringArray(int num_val, char **val)
     builder = g_variant_builder_new(G_VARIANT_TYPE("a(s)"));
     for (int i = 0; i < num_val; i++)
     {
-        // g_message("%s", val[i]);
         g_variant_builder_add(builder, "(s)", val[i]);
     }
 
     if (num_val == 0)
         g_variant_builder_add(builder, "(s)", "NA");
 
-    values = g_variant_new("a(s)", builder);
+    values = g_variant_builder_end(builder);
     return values;
 }
 

cpdb/cpdb.h

@@ -43,7 +43,6 @@ extern "C" {
 #define CPDB_PRINTER_ADDED_ARGS "(sssssbss)"
 #define CPDB_JOB_ARGS "(ssssssi)"
 #define CPDB_JOB_ARRAY_ARGS "a(ssssssi)"
-#define cpdbNewCStringArray(x) ((char **)(malloc(sizeof(char *) * x)))
 
 typedef enum {
     CPDB_DEBUG_LEVEL_DEBUG,
@@ -58,6 +57,11 @@ typedef enum {
  */
 void cpdbInit();
 
+/**
+ * Get an array of C styled strings (char *)
+ */
+char **cpdbNewCStringArray(int num_elems);
+
 /**
  * Convert string to gboolean.
  */
@@ -218,8 +222,8 @@ GVariant *cpdbPackMediaArray(int num_val, int (*margins)[4]);
 #define CPDB_QUALITY_HIGH               N_("high")
 
 #define CPDB_SIDES_ONE_SIDED            N_("one-sided")
-#define CPDB_SIDES_TWO_SIDED_SHORT      N_("two-sided-short")
-#define CPDB_SIDES_TWO_SIDED_LONG       N_("two-sided-long")
+#define CPDB_SIDES_TWO_SIDED_SHORT      N_("two-sided-short-edge")
+#define CPDB_SIDES_TWO_SIDED_LONG       N_("two-sided-long-edge")
 
 #define CPDB_ORIENTATION_PORTRAIT       N_("3")
 #define CPDB_ORIENTATION_LANDSCAPE      N_("4")