Handle filter open and read errors better.

If the user's color filter file exists but can't be opened, fail and
return a failure indication.

If it doesn't exist, just try reading the global color filter file.

If *that* exists but can't be opened, fail and return a failure
indication; if it doesn't exist, don't treat that as an error.

Change-Id: I5ebdc0eb1ada3cc190990be9228639304f32147e
Reviewed-on: https://code.wireshark.org/review/22054
Reviewed-by: Guy Harris <guy@alum.mit.edu>

1 files changed, 67 insertions, 70 deletions

diff --git a/epan/color_filters.c b/epan/color_filters.c
index 6f7e40d51c..3a184bb3a8 100644
--- a/epan/color_filters.c
+++ b/epan/color_filters.c
@@ -42,12 +42,11 @@
 #include <epan/prefs.h>
 #include <epan/epan_dissect.h>
 
-
 #define RED_COMPONENT(x)   (guint16) (((((x) >> 16) & 0xff) * 65535 / 255))
 #define GREEN_COMPONENT(x) (guint16) (((((x) >>  8) & 0xff) * 65535 / 255))
 #define BLUE_COMPONENT(x)  (guint16) ( (((x)        & 0xff) * 65535 / 255))
 
-static gboolean read_users_filters(GSList **cfl, gchar** err_msg, color_filter_add_cb_func add_cb);
+static int read_filters_file(const gchar *path, FILE *f, gpointer user_data, color_filter_add_cb_func add_cb);
 
 /* the currently active filters */
 static GSList *color_filter_list = NULL;
@@ -293,32 +292,65 @@ color_filter_list_clone(GSList *cfl)
     return new_list;
 }
 
-/* Initialize the filter structures (reading from file) for general running, including app startup */
-gboolean
-color_filters_init(gchar** err_msg, color_filter_add_cb_func add_cb)
+static gboolean
+color_filters_get(gchar** err_msg, color_filter_add_cb_func add_cb)
 {
-    /* delete all currently existing filters */
-    color_filter_list_delete(&color_filter_list);
+    gchar    *path;
+    FILE     *f;
+    int       ret;
 
     /* start the list with the temporary colorizing rules */
     color_filters_add_tmp(&color_filter_list);
 
-    /* try to read the users filters */
-    if (!read_users_filters(&color_filter_list, err_msg, add_cb)) {
-        gchar* local_err_msg = NULL;
-
-        /* if that failed, try to read the global filters */
-        if (!color_filters_read_globals(&color_filter_list, &local_err_msg, add_cb)) {
-            /* Show the first error */
-            g_free(local_err_msg);
-        }
+    /*
+     * Try to get the user's filters.
+     *
+     * Get the path for the file that would have their filters, and
+     * try to open it.
+     */
+    path = get_persconffile_path("colorfilters", TRUE);
+    if ((f = ws_fopen(path, "r")) == NULL) {
+        if (errno != ENOENT) {
+            /* Error trying to open the file; give up. */
+            *err_msg = g_strdup_printf("Could not open filter file\n\"%s\": %s.", path,
+                                       g_strerror(errno));
+            g_free(path);
+            return FALSE;
+	}
+        /* They don't have any filters; try to read the global filters */
+        g_free(path);
+        return color_filters_read_globals(&color_filter_list, err_msg, add_cb);
+    }
 
-        return (*err_msg == NULL);
+    /*
+     * We've opened it; try to read it.
+     */
+    ret = read_filters_file(path, f, &color_filter_list, add_cb);
+    if (ret != 0) {
+        *err_msg = g_strdup_printf("Error reading filter file\n\"%s\": %s.",
+                                   path, g_strerror(errno));
+        fclose(f);
+        g_free(path);
+        return FALSE;
     }
 
+    /* Success. */
+    fclose(f);
+    g_free(path);
     return TRUE;
 }
 
+/* Initialize the filter structures (reading from file) for general running, including app startup */
+gboolean
+color_filters_init(gchar** err_msg, color_filter_add_cb_func add_cb)
+{
+    /* delete all currently existing filters */
+    color_filter_list_delete(&color_filter_list);
+
+    /* now try to construct the filters list */
+    return color_filters_get(err_msg, add_cb);
+}
+
 gboolean
 color_filters_reload(gchar** err_msg, color_filter_add_cb_func add_cb)
 {
@@ -327,22 +359,8 @@ color_filters_reload(gchar** err_msg, color_filter_add_cb_func add_cb)
     color_filter_deleted_list = g_slist_concat(color_filter_deleted_list, color_filter_list);
     color_filter_list = NULL;
 
-    /* start the list with the temporary colorizing rules */
-    color_filters_add_tmp(&color_filter_list);
-
-    /* try to read the users filters */
-    if (!read_users_filters(&color_filter_list, err_msg, add_cb)) {
-        gchar* local_err_msg = NULL;
-
-        /* if that failed, try to read the global filters */
-        if (!color_filters_read_globals(&color_filter_list, &local_err_msg, add_cb)) {
-            /* Show the first error */
-            g_free(local_err_msg);
-        }
-
-        return (*err_msg == NULL);
-    }
-    return TRUE;
+    /* now try to construct the filters list */
+    return color_filters_get(err_msg, add_cb);
 }
 
 void
@@ -674,39 +692,6 @@ read_filters_file(const gchar *path, FILE *f, gpointer user_data, color_filter_a
     return ret;
 }
 
-/* read filters from the user's filter file */
-static gboolean
-read_users_filters(GSList **cfl, gchar** err_msg, color_filter_add_cb_func add_cb)
-{
-    gchar    *path;
-    FILE     *f;
-    int       ret;
-
-    /* decide what file to open (from dfilter code) */
-    path = get_persconffile_path("colorfilters", TRUE);
-    if ((f = ws_fopen(path, "r")) == NULL) {
-        if (errno != ENOENT) {
-            *err_msg = g_strdup_printf("Could not open filter file\n\"%s\": %s.", path,
-                          g_strerror(errno));
-        }
-        g_free(path);
-        return FALSE;
-    }
-
-    ret = read_filters_file(path, f, cfl, add_cb);
-    if (ret != 0) {
-        *err_msg = g_strdup_printf("Error reading filter file\n\"%s\": %s.",
-                                   path, g_strerror(errno));
-        fclose(f);
-        g_free(path);
-        return FALSE;
-    }
-
-    fclose(f);
-    g_free(path);
-    return TRUE;
-}
-
 /* read filters from the filter file */
 gboolean
 color_filters_read_globals(gpointer user_data, gchar** err_msg, color_filter_add_cb_func add_cb)
@@ -715,15 +700,27 @@ color_filters_read_globals(gpointer user_data, gchar** err_msg, color_filter_add
     FILE     *f;
     int       ret;
 
-    /* decide what file to open (from dfilter code) */
+    /*
+     * Try to get the global filters.
+     *
+     * Get the path for the file that would have the global filters, and
+     * try to open it.
+     */
     path = get_datafile_path("colorfilters");
     if ((f = ws_fopen(path, "r")) == NULL) {
         if (errno != ENOENT) {
+            /* Error trying to open the file; give up. */
             *err_msg = g_strdup_printf("Could not open global filter file\n\"%s\": %s.", path,
-                          g_strerror(errno));
+                                       g_strerror(errno));
+            g_free(path);
+            return FALSE;
         }
-        g_free(path);
-        return FALSE;
+
+        /*
+         * There is no global filter file; treat that as equivalent to
+         * that file existing bug being empty, and say we succeeded.
+         */
+        return TRUE;
     }
 
     ret = read_filters_file(path, f, user_data, add_cb);