Pull the error reporting into {read,save}_filter_list.

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

10 files changed, 72 insertions, 192 deletions

diff --git a/sharkd.c b/sharkd.c
index 30df444d84..3b10e89f79 100644
--- a/sharkd.c
+++ b/sharkd.c
@@ -122,9 +122,7 @@ main(int argc, char *argv[])
   GString             *runtime_info_str;
   char                *init_progfile_dir_error;
 
-  char                *cf_path;
   char                *err_msg = NULL;
-  int                  cf_open_errno;
   e_prefs             *prefs_p;
   int                  ret = EXIT_SUCCESS;
 
@@ -208,12 +206,7 @@ main(int argc, char *argv[])
 
   prefs_p = read_prefs();
 
-  read_filter_list(CFILTER_LIST, &cf_path, &cf_open_errno);
-  if (cf_path != NULL) {
-      cmdarg_err("Could not open your capture filter file\n\"%s\": %s.",
-          cf_path, g_strerror(cf_open_errno));
-      g_free(cf_path);
-  }
+  read_filter_list(CFILTER_LIST);
 
   if (!color_filters_init(&err_msg, NULL)) {
      fprintf(stderr, "color_filters_init() failed %s\n", err_msg);
diff --git a/tshark.c b/tshark.c
index 19af646675..6a7e345507 100644
--- a/tshark.c
+++ b/tshark.c
@@ -665,8 +665,6 @@ main(int argc, char *argv[])
   WSADATA              wsaData;
 #endif  /* _WIN32 */
 
-  char                *cf_path;
-  int                  cf_open_errno;
   int                  err;
   volatile int         exit_status = EXIT_SUCCESS;
 #ifdef HAVE_LIBPCAP
@@ -1009,12 +1007,7 @@ main(int argc, char *argv[])
 
   prefs_p = read_prefs();
 
-  read_filter_list(CFILTER_LIST, &cf_path, &cf_open_errno);
-  if (cf_path != NULL) {
-      cmdarg_err("Could not open your capture filter file\n\"%s\": %s.",
-          cf_path, g_strerror(cf_open_errno));
-      g_free(cf_path);
-  }
+  read_filter_list(CFILTER_LIST);
 
   /*
    * Read the files that enable and disable protocols and heuristic
diff --git a/ui/filter_files.c b/ui/filter_files.c
index e58072221b..092b8afc79 100644
--- a/ui/filter_files.c
+++ b/ui/filter_files.c
@@ -31,6 +31,7 @@
 #include <wsutil/file_util.h>
 #include <wsutil/filesystem.h>
 #include <wsutil/glib-compat.h>
+#include <wsutil/report_message.h>
 
 #include "ui/filter_files.h"
 
@@ -72,10 +73,7 @@ static GList *display_edited_filters = NULL;
 /*
  * Read in a list of filters.
  *
- * On success, "*pref_path_return" is set to NULL.
- * On error, "*pref_path_return" is set to point to the pathname of
- * the file we tried to read - it should be freed by our caller -
- * and "*errno_return" is set to the error.
+ * On error, report the error via the UI.
  */
 
 #define INIT_BUF_SIZE  128
@@ -163,10 +161,9 @@ getc_crlf(FILE *ff)
 }
 
 void
-read_filter_list(filter_list_type_t list_type, char **pref_path_return,
-    int *errno_return)
+read_filter_list(filter_list_type_t list_type)
 {
-  const char *ff_name;
+  const char *ff_name, *ff_description;
   char       *ff_path;
   FILE       *ff;
   GList      **flpp;
@@ -176,17 +173,17 @@ read_filter_list(filter_list_type_t list_type, char **pref_path_return,
   int         filt_name_index, filt_expr_index;
   int         line = 1;
 
-  *pref_path_return = NULL;   /* assume no error */
-
   switch (list_type) {
 
   case CFILTER_LIST:
     ff_name = CFILTER_FILE_NAME;
+    ff_description = "capture";
     flpp = &capture_filters;
     break;
 
   case DFILTER_LIST:
     ff_name = DFILTER_FILE_NAME;
+    ff_description = "display";
     flpp = &display_filters;
     break;
 
@@ -205,8 +202,9 @@ read_filter_list(filter_list_type_t list_type, char **pref_path_return,
       /*
        * No.  Just give up.
        */
-      *pref_path_return = ff_path;
-      *errno_return = errno;
+      report_warning("Could not open your %s filter file\n\"%s\": %s.",
+                     ff_description, ff_path, g_strerror(errno));
+      g_free(ff_path);
       return;
     }
 
@@ -224,31 +222,30 @@ read_filter_list(filter_list_type_t list_type, char **pref_path_return,
       /*
        * Did that fail because the file didn't exist?
        */
-        if (errno != ENOENT) {
+      if (errno != ENOENT) {
         /*
          * No.  Just give up.
          */
-          *pref_path_return = ff_path;
-          *errno_return = errno;
-          return;
-        }
+        report_warning("Could not open your %s filter file\n\"%s\": %s.",
+                       ff_description, ff_path, g_strerror(errno));
+        g_free(ff_path);
+        return;
+      }
 
       /*
        * Try to open the global "cfilters/dfilters" file */
       g_free(ff_path);
       ff_path = get_datafile_path(ff_name);
       if ((ff = ws_fopen(ff_path, "r")) == NULL) {
-
         /*
          * Well, that didn't work, either.  Just give up.
-         * Return an error if the file existed but we couldn't open it.
+         * Report an error if the file existed but we couldn't open it.
          */
         if (errno != ENOENT) {
-          *pref_path_return = ff_path;
-          *errno_return = errno;
-        } else {
-          g_free(ff_path);
+          report_warning("Could not open your %s filter file\n\"%s\": %s.",
+                         ff_description, ff_path, g_strerror(errno));
         }
+        g_free(ff_path);
         return;
       }
     }
@@ -401,10 +398,10 @@ read_filter_list(filter_list_type_t list_type, char **pref_path_return,
     *flpp = add_filter_entry(*flpp, filt_name, filt_expr);
   }
   if (ferror(ff)) {
-    *pref_path_return = ff_path;
-    *errno_return = errno;
-  } else
-    g_free(ff_path);
+    report_warning("Error reading your %s filter file\n\"%s\": %s.",
+                   ff_description, ff_path, g_strerror(errno));
+  }
+  g_free(ff_path);
   fclose(ff);
   g_free(filt_name);
   g_free(filt_expr);
@@ -499,16 +496,13 @@ remove_from_filter_list(filter_list_type_t list_type, GList *fl_entry)
 /*
  * Write out a list of filters.
  *
- * On success, "*pref_path_return" is set to NULL.
- * On error, "*pref_path_return" is set to point to the pathname of
- * the file we tried to read - it should be freed by our caller -
- * and "*errno_return" is set to the error.
+ * On error, report the error via the UI.
  */
 void
-save_filter_list(filter_list_type_t list_type, char **pref_path_return,
-    int *errno_return)
+save_filter_list(filter_list_type_t list_type)
 {
-  const gchar *ff_name;
+  char        *pf_dir_path;
+  const gchar *ff_name, *ff_description;
   gchar       *ff_path, *ff_path_new;
   GList       *fl;
   GList       *flpp;
@@ -516,17 +510,17 @@ save_filter_list(filter_list_type_t list_type, char **pref_path_return,
   FILE        *ff;
   guchar      *p, c;
 
-  *pref_path_return = NULL;   /* assume no error */
-
   switch (list_type) {
 
   case CFILTER_LIST:
     ff_name = CFILTER_FILE_NAME;
+    ff_description = "capture";
     fl = capture_filters;
     break;
 
   case DFILTER_LIST:
     ff_name = DFILTER_FILE_NAME;
+    ff_description = "display";
     fl = display_filters;
     break;
 
@@ -535,6 +529,15 @@ save_filter_list(filter_list_type_t list_type, char **pref_path_return,
     return;
   }
 
+  /* Create the directory that holds personal configuration files,
+     if necessary.  */
+  if (create_persconffile_dir(&pf_dir_path) == -1) {
+    report_failure("Can't create directory\n\"%s\"\nfor filter files: %s.",
+                   pf_dir_path, g_strerror(errno));
+    g_free(pf_dir_path);
+    return;
+  }
+
   ff_path = get_persconffile_path(ff_name, TRUE);
 
   /* Write to "XXX.new", and rename if that succeeds.
@@ -543,9 +546,11 @@ save_filter_list(filter_list_type_t list_type, char **pref_path_return,
   ff_path_new = g_strdup_printf("%s.new", ff_path);
 
   if ((ff = ws_fopen(ff_path_new, "w")) == NULL) {
-    *pref_path_return = ff_path;
-    *errno_return = errno;
+    /* We had an error saving the filter. */
+    report_failure("Error saving your %s filter file\nCouldn't open \"%s\": %s.",
+                   ff_description, ff_path_new, g_strerror(errno));
     g_free(ff_path_new);
+    g_free(ff_path);
     return;
   }
   flpp = g_list_first(fl);
@@ -568,20 +573,22 @@ save_filter_list(filter_list_type_t list_type, char **pref_path_return,
     /* Write out the filter expression and a newline. */
     fprintf(ff, "%s\n", filt->strval);
     if (ferror(ff)) {
-      *pref_path_return = ff_path;
-      *errno_return = errno;
+      report_failure("Error saving your %s filter file\nWrite to \"%s\" failed: %s.",
+                     ff_description, ff_path_new, g_strerror(errno));
       fclose(ff);
       ws_unlink(ff_path_new);
       g_free(ff_path_new);
+      g_free(ff_path);
       return;
     }
     flpp = flpp->next;
   }
   if (fclose(ff) == EOF) {
-    *pref_path_return = ff_path;
-    *errno_return = errno;
+    report_failure("Error saving your %s filter file\nWrite to \"%s\" failed: %s.",
+                   ff_description, ff_path_new, g_strerror(errno));
     ws_unlink(ff_path_new);
     g_free(ff_path_new);
+    g_free(ff_path);
     return;
   }
 
@@ -601,19 +608,21 @@ save_filter_list(filter_list_type_t list_type, char **pref_path_return,
     /* It failed for some reason other than "it's not there"; if
        it's not there, we don't need to remove it, so we just
        drive on. */
-    *pref_path_return = ff_path;
-    *errno_return = errno;
+    report_failure("Error saving your %s filter file\nCouldn't remove \"%s\": %s.",
+                   ff_description, ff_path, g_strerror(errno));
     ws_unlink(ff_path_new);
     g_free(ff_path_new);
+    g_free(ff_path);
     return;
   }
 #endif
 
   if (ws_rename(ff_path_new, ff_path) < 0) {
-    *pref_path_return = ff_path;
-    *errno_return = errno;
+    report_failure("Error saving your %s filter file\nCouldn't rename \"%s\" to \"%s\": %s.",
+                   ff_description, ff_path_new, ff_path, g_strerror(errno));
     ws_unlink(ff_path_new);
     g_free(ff_path_new);
+    g_free(ff_path);
     return;
   }
   g_free(ff_path_new);
diff --git a/ui/filter_files.h b/ui/filter_files.h
index f953b2f506..57445d4fdc 100644
--- a/ui/filter_files.h
+++ b/ui/filter_files.h
@@ -48,13 +48,9 @@ typedef struct {
 /*
  * Read in a list of filters.
  *
- * On success, "*pref_path_return" is set to NULL.
- * On error, "*pref_path_return" is set to point to the pathname of
- * the file we tried to read - it should be freed by our caller -
- * and "*errno_return" is set to the error.
+ * On error, report the error via the UI.
  */
-void read_filter_list(filter_list_type_t list_type, char **pref_path_return,
-                      int *errno_return);
+void read_filter_list(filter_list_type_t list_type);
 
 /*
  * Get a pointer to the first entry in a filter list.
@@ -76,13 +72,9 @@ void remove_from_filter_list(filter_list_type_t list, GList *fl_entry);
 /*
  * Write out a list of filters.
  *
- * On success, "*pref_path_return" is set to NULL.
- * On error, "*pref_path_return" is set to point to the pathname of
- * the file we tried to read - it should be freed by our caller -
- * and "*errno_return" is set to the error.
+ * On error, report the error via the UI.
  */
-void save_filter_list(filter_list_type_t list_type, char **pref_path_return,
-                      int *errno_return);
+void save_filter_list(filter_list_type_t list_type);
 
 /*
  * Clone the filter list so it can be edited. GTK+ only.
diff --git a/ui/gtk/filter_dlg.c b/ui/gtk/filter_dlg.c
index 7604f078e7..cf699d8467 100644
--- a/ui/gtk/filter_dlg.c
+++ b/ui/gtk/filter_dlg.c
@@ -798,49 +798,24 @@ filter_apply(GtkWidget *main_w, gboolean destroy)
 static void
 filter_dlg_save(filter_list_type_t list_type)
 {
-    char *pf_dir_path;
-    char *f_path;
-    int f_save_errno;
-    const char *filter_type;
-
     switch (list_type) {
 
     case CFILTER_EDITED_LIST:
-        filter_type = "capture";
-                list_type = CFILTER_LIST;
-                copy_filter_list(CFILTER_LIST, CFILTER_EDITED_LIST);
+        list_type = CFILTER_LIST;
+        copy_filter_list(CFILTER_LIST, CFILTER_EDITED_LIST);
         break;
 
     case DFILTER_EDITED_LIST:
-        filter_type = "display";
-                list_type = DFILTER_LIST;
-                copy_filter_list(DFILTER_LIST, DFILTER_EDITED_LIST);
+        list_type = DFILTER_LIST;
+        copy_filter_list(DFILTER_LIST, DFILTER_EDITED_LIST);
         break;
 
     default:
         g_assert_not_reached();
-        filter_type = NULL;
         break;
     }
 
-    /* Create the directory that holds personal configuration files,
-       if necessary.  */
-    if (create_persconffile_dir(&pf_dir_path) == -1) {
-        simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-            "Can't create directory\n\"%s\"\nfor filter files: %s.",
-            pf_dir_path, g_strerror(errno));
-        g_free(pf_dir_path);
-        return;
-    }
-
-    save_filter_list(list_type, &f_path, &f_save_errno);
-    if (f_path != NULL) {
-        /* We had an error saving the filter. */
-        simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-            "Could not save to your %s filter file\n\"%s\": %s.",
-            filter_type, f_path, g_strerror(f_save_errno));
-        g_free(f_path);
-    }
+    save_filter_list(list_type);
 }
 
 
diff --git a/ui/gtk/main.c b/ui/gtk/main.c
index 2e0d53a6f7..1516fbadce 100644
--- a/ui/gtk/main.c
+++ b/ui/gtk/main.c
@@ -1905,8 +1905,6 @@ get_wireshark_runtime_info(GString *str)
 static e_prefs *
 read_configuration_files(void)
 {
-    int                  cf_open_errno, df_open_errno;
-    char                *cf_path, *df_path;
     e_prefs             *prefs_p;
 
     /* load the decode as entries of this profile */
@@ -1923,22 +1921,10 @@ read_configuration_files(void)
 #endif
 
     /* Read the capture filter file. */
-    read_filter_list(CFILTER_LIST, &cf_path, &cf_open_errno);
-    if (cf_path != NULL) {
-        simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
-                      "Could not open your capture filter file\n\"%s\": %s.",
-                      cf_path, g_strerror(cf_open_errno));
-        g_free(cf_path);
-    }
+    read_filter_list(CFILTER_LIST);
 
     /* Read the display filter file. */
-    read_filter_list(DFILTER_LIST, &df_path, &df_open_errno);
-    if (df_path != NULL) {
-        simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
-                      "Could not open your display filter file\n\"%s\": %s.",
-                      df_path, g_strerror(df_open_errno));
-        g_free(df_path);
-    }
+    read_filter_list(DFILTER_LIST);
 
     /*
      * Read the files that enable and disable protocols and heuristic
diff --git a/ui/qt/capture_filter_edit.cpp b/ui/qt/capture_filter_edit.cpp
index 89e2bd8df4..083b1df119 100644
--- a/ui/qt/capture_filter_edit.cpp
+++ b/ui/qt/capture_filter_edit.cpp
@@ -506,18 +506,7 @@ void CaptureFilterEdit::removeFilter()
         }
     }
 
-    char *f_path;
-    int f_save_errno;
-
-    save_filter_list(CFILTER_LIST, &f_path, &f_save_errno);
-    if (f_path != NULL) {
-        // We had an error saving the filter.
-        QString warning_title = tr("Unable to save capture filter settings.");
-        QString warning_msg = tr("Could not save to your capture filter file\n\"%1\": %2.").arg(f_path).arg(g_strerror(f_save_errno));
-
-        QMessageBox::warning(this, warning_title, warning_msg, QMessageBox::Ok);
-        g_free(f_path);
-    }
+    save_filter_list(CFILTER_LIST);
 
     updateBookmarkMenu();
 }
diff --git a/ui/qt/display_filter_edit.cpp b/ui/qt/display_filter_edit.cpp
index 97b035fc5d..eb44075462 100644
--- a/ui/qt/display_filter_edit.cpp
+++ b/ui/qt/display_filter_edit.cpp
@@ -537,18 +537,7 @@ void DisplayFilterEdit::removeFilter()
         }
     }
 
-    char *f_path;
-    int f_save_errno;
-
-    save_filter_list(DFILTER_LIST, &f_path, &f_save_errno);
-    if (f_path != NULL) {
-        // We had an error saving the filter.
-        QString warning_title = tr("Unable to save display filter settings.");
-        QString warning_msg = tr("Could not save to your display filter file\n\"%1\": %2.").arg(f_path).arg(g_strerror(f_save_errno));
-
-        QMessageBox::warning(this, warning_title, warning_msg, QMessageBox::Ok);
-        g_free(f_path);
-    }
+    save_filter_list(DFILTER_LIST);
 
     updateBookmarkMenu();
 }
diff --git a/ui/qt/filter_dialog.cpp b/ui/qt/filter_dialog.cpp
index 9652a327dc..dd05ead167 100644
--- a/ui/qt/filter_dialog.cpp
+++ b/ui/qt/filter_dialog.cpp
@@ -203,39 +203,7 @@ void FilterDialog::on_buttonBox_accepted()
         ++it;
     }
 
-    char *pf_dir_path;
-    char *f_path;
-    int f_save_errno;
-
-    /* Create the directory that holds personal configuration files,
-       if necessary.  */
-    if (create_persconffile_dir(&pf_dir_path) == -1) {
-        QMessageBox::warning(this, tr("Unable to create profile directory."),
-                tr("Unable to create directory\n\"%1\"\nfor filter files: %2.")
-                             .arg(pf_dir_path)
-                             .arg(g_strerror(errno)),
-                QMessageBox::Ok);
-        g_free(pf_dir_path);
-        return;
-    }
-
-    save_filter_list(fl_type, &f_path, &f_save_errno);
-    if (f_path != NULL) {
-        /* We had an error saving the filter. */
-        QString warning_title;
-        QString warning_msg;
-        if (fl_type == CFILTER_LIST) {
-            warning_title = tr("Unable to save capture filter settings.");
-            warning_msg = tr("Could not save to your capture filter file\n\"%1\": %2.")
-              .arg(f_path).arg(g_strerror(f_save_errno));
-        } else {
-            warning_title = tr("Unable to save display filter settings.");
-            warning_msg = tr("Could not save to your display filter file\n\"%1\": %2.")
-              .arg(f_path).arg(g_strerror(f_save_errno));
-        }
-        QMessageBox::warning(this, warning_title, warning_msg, QMessageBox::Ok);
-        g_free(f_path);
-    }
+    save_filter_list(fl_type);
 
     if (filter_type_ == CaptureFilter) {
         wsApp->emitAppSignal(WiresharkApplication::CaptureFilterListChanged);
diff --git a/ui/qt/wireshark_application.cpp b/ui/qt/wireshark_application.cpp
index 7f25a2138f..3345bf4bd3 100644
--- a/ui/qt/wireshark_application.cpp
+++ b/ui/qt/wireshark_application.cpp
@@ -1095,8 +1095,6 @@ void WiresharkApplication::allSystemsGo()
 
 _e_prefs *WiresharkApplication::readConfigurationFiles(bool reset)
 {
-    int                  cf_open_errno, df_open_errno;
-    char                *cf_path, *df_path;
     e_prefs             *prefs_p;
 
     if (reset) {
@@ -1123,22 +1121,10 @@ _e_prefs *WiresharkApplication::readConfigurationFiles(bool reset)
 #endif
 
     /* Read the capture filter file. */
-    read_filter_list(CFILTER_LIST, &cf_path, &cf_open_errno);
-    if (cf_path != NULL) {
-        simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
-                      "Could not open your capture filter file\n\"%s\": %s.", cf_path,
-                      g_strerror(cf_open_errno));
-        g_free(cf_path);
-    }
+    read_filter_list(CFILTER_LIST);
 
     /* Read the display filter file. */
-    read_filter_list(DFILTER_LIST, &df_path, &df_open_errno);
-    if (df_path != NULL) {
-        simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
-                      "Could not open your display filter file\n\"%s\": %s.", df_path,
-                      g_strerror(df_open_errno));
-        g_free(df_path);
-    }
+    read_filter_list(DFILTER_LIST);
 
     /*
      * Read the files that enable and disable protocols and heuristic