Qt: Provide both file save and open preferences

This is a breaking change.

prefs_register_filename_preference hasn't been differentiating
between files to be saved and ones to be opened.

On GTK, a neutral dialog is used, so no problems there.
On Qt, a save dialog has been always used, even in dissectors that
were reading configuration files without modification.

prefs_register_filename_preference now takes an argument to indicate
whether UI could be a save dialog with a warning on overwriting
a file, or whether it's a general purpose open file dialog.

Qt now does this. Previously no warning was shown on overwriting a file,
so it may be used for opening files too without irritating the user.
This has been changed, as non-destructive reads should now use
the open dialog.

Dissectors were changed accordingly.

Change-Id: I9087fefa5ee7ca58de0775d4fe2c0fdcfa3a3018
Reviewed-on: https://code.wireshark.org/review/21086
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

doc/README.dissector

@@ -3063,10 +3063,11 @@ routines -
     /* Register a preference with a file name (string) value.
     * File name preferences are basically like string preferences
     * except that the GUI gives the user the ability to browse for the
-    * file.
+    * file. Set for_writing TRUE to show a Save dialog instead of normal Open.
     */
     void prefs_register_filename_preference(module_t *module, const char *name,
-        const char *title, const char *description, char **var)
+        const char *title, const char *description, char **var,
+        gboolean for_writing)
 
     /* Register a preference with a range of unsigned integers (e.g.,
      * "1-20,30-40").

epan/dissectors/asn1/kerberos/packet-kerberos-template.c

@@ -2284,7 +2284,7 @@ void proto_register_kerberos(void) {
 	prefs_register_filename_preference(krb_module, "file",
 				   "Kerberos keytab file",
 				   "The keytab file containing all the secrets",
-				   &keytab_filename);
+				   &keytab_filename, FALSE);
 #endif
 
 }

epan/dissectors/packet-dtls.c

@@ -1921,7 +1921,7 @@ proto_register_dtls(void)
     prefs_register_filename_preference(dtls_module, "debug_file", "DTLS debug file",
                                        "redirect dtls debug to file name; leave empty to disable debug, "
                                        "use \"" SSL_DEBUG_USE_STDERR "\" to redirect output to stderr\n",
-                                       &dtls_debug_file_name);
+                                       &dtls_debug_file_name, TRUE);
 
     prefs_register_string_preference(dtls_module, "keys_list", "RSA keys list (deprecated)",
                                      "Semicolon-separated list of private RSA keys used for DTLS decryption. "

epan/dissectors/packet-giop.c

@@ -5557,7 +5557,7 @@ proto_register_giop (void)
                                  10, &giop_max_message_size);
 
   prefs_register_filename_preference(giop_module, "ior_txt", "Stringified IORs",
-    "File containing stringified IORs, one per line.", &giop_ior_file);
+    "File containing stringified IORs, one per line.", &giop_ior_file, FALSE);
 
   /*
    * Init the giop user module hash tables here, as giop users

epan/dissectors/packet-kerberos.c

@@ -5379,7 +5379,7 @@ void proto_register_kerberos(void) {
 	prefs_register_filename_preference(krb_module, "file",
 				   "Kerberos keytab file",
 				   "The keytab file containing all the secrets",
-				   &keytab_filename);
+				   &keytab_filename, FALSE);
 #endif
 
 }

epan/dissectors/packet-snort.c

@@ -1471,11 +1471,11 @@ proto_register_snort(void)
     prefs_register_filename_preference(snort_module, "binary",
                                        "Snort binary",
                                        "The name of the snort binary file to run",
-                                       &pref_snort_binary_filename);
+                                       &pref_snort_binary_filename, FALSE);
     prefs_register_filename_preference(snort_module, "config",
                                        "Configuration filename",
                                        "The name of the file containing the snort IDS configuration.  Typically snort.conf",
-                                       &pref_snort_config_filename);
+                                       &pref_snort_config_filename, FALSE);
 
     prefs_register_bool_preference(snort_module, "show_rule_set_stats",
                                    "Show rule stats in protocol tree",

epan/dissectors/packet-ssl-utils.c

@@ -8463,7 +8463,7 @@ ssl_common_register_options(module_t *module, ssl_common_options_t *options)
              "<CRAND> = The Client's random number from the ClientHello message\n"
              "\n"
              "(All fields are in hex notation)",
-             &(options->keylog_filename));
+             &(options->keylog_filename), FALSE);
 }
 
 void

epan/dissectors/packet-ssl.c

@@ -4238,7 +4238,7 @@ proto_register_ssl(void)
         prefs_register_filename_preference(ssl_module, "debug_file", "SSL debug file",
             "Redirect SSL debug to the file specified. Leave empty to disable debugging "
             "or use \"" SSL_DEBUG_USE_STDERR "\" to redirect output to stderr.",
-            &ssl_debug_file_name);
+            &ssl_debug_file_name, TRUE);
 
         prefs_register_string_preference(ssl_module, "keys_list", "RSA keys list (deprecated)",
              "Semicolon-separated list of private RSA keys used for SSL decryption. "

epan/prefs-int.h

@@ -102,13 +102,14 @@ struct pref_custom_cbs {
 #define PREF_RANGE            (1u << 4)
 #define PREF_STATIC_TEXT      (1u << 5)
 #define PREF_UAT              (1u << 6)
-#define PREF_FILENAME    (1u << 7)
+#define PREF_SAVE_FILENAME    (1u << 7)
 #define PREF_COLOR            (1u << 8) /* XXX - These are only supported for "internal" (non-protocol) */
 #define PREF_CUSTOM           (1u << 9) /* use and not as a generic protocol preference */
 #define PREF_OBSOLETE         (1u << 10)
 #define PREF_DIRNAME          (1u << 11)
 #define PREF_DECODE_AS_UINT   (1u << 12)     /* XXX - These are only supported for "internal" (non-protocol) */
 #define PREF_DECODE_AS_RANGE  (1u << 13) /* use and not as a generic protocol preference */
+#define PREF_OPEN_FILENAME    (1u << 14)
 
 typedef enum {
 	GUI_ALL,

epan/prefs.c

@@ -352,7 +352,8 @@ free_pref(gpointer data, gpointer user_data _U_)
     case PREF_COLOR:
         break;
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         g_free(*pref->varp.string);
         *pref->varp.string = NULL;
@@ -1443,11 +1444,11 @@ DIAG_ON(cast-qual)
 void
 prefs_register_filename_preference(module_t *module, const char *name,
                                    const char *title, const char *description,
-                                   const char **var)
+                                   const char **var, gboolean for_writing)
 {
 DIAG_OFF(cast-qual)
-    register_string_like_preference(module, name, title, description,
+    register_string_like_preference(module, name, title, description, (char **)var,
-                                    (char **)var, PREF_FILENAME, NULL, FALSE);
+                                    for_writing ? PREF_SAVE_FILENAME : PREF_OPEN_FILENAME, NULL, FALSE);
 DIAG_ON(cast-qual)
 }
 
@@ -1914,7 +1915,8 @@ pref_stash(pref_t *pref, gpointer unused _U_)
         break;
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         g_free(pref->stashed_val.string);
         pref->stashed_val.string = g_strdup(*pref->varp.string);
@@ -1998,7 +2000,8 @@ pref_unstash(pref_t *pref, gpointer unstash_data_p)
         break;
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         if (strcmp(*pref->varp.string, pref->stashed_val.string) != 0) {
             unstash_data->module->prefs_changed = TRUE;
@@ -2098,7 +2101,8 @@ reset_stashed_pref(pref_t *pref) {
         break;
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         g_free(pref->stashed_val.string);
         pref->stashed_val.string = g_strdup(pref->default_val.string);
@@ -2141,7 +2145,8 @@ pref_clean_stash(pref_t *pref, gpointer unused _U_)
         break;
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         if (pref->stashed_val.string != NULL) {
             g_free(pref->stashed_val.string);
@@ -3450,7 +3455,7 @@ prefs_register_modules(void)
 
     register_string_like_preference(printing, "file", "File",
         "This is the file that gets written to when the destination is set to \"file\"",
-        &prefs.pr_file, PREF_FILENAME, NULL, TRUE);
+        &prefs.pr_file, PREF_SAVE_FILENAME, NULL, TRUE);
 
     /* Statistics */
     stats_module = prefs_register_module(NULL, "statistics", "Statistics",
@@ -4108,7 +4113,8 @@ reset_pref(pref_t *pref)
         break;
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         reset_string_like_preference(pref);
         break;
@@ -5751,7 +5757,8 @@ set_pref(gchar *pref_name, const gchar *value, void *private_data _U_,
             break;
 
         case PREF_STRING:
-        case PREF_FILENAME:
+        case PREF_SAVE_FILENAME:
+        case PREF_OPEN_FILENAME:
         case PREF_DIRNAME:
             containing_module->prefs_changed |= prefs_set_string_value(pref, value, pref_current);
             break;
@@ -5901,7 +5908,8 @@ prefs_pref_type_name(pref_t *pref)
         type_name = "String";
         break;
 
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
         type_name = "Filename";
         break;
 
@@ -6002,7 +6010,8 @@ prefs_pref_type_description(pref_t *pref)
         type_desc = "A string";
         break;
 
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
         type_desc = "A path to a file";
         break;
 
@@ -6086,7 +6095,8 @@ prefs_pref_is_default(pref_t *pref)
         break;
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         if (!(g_strcmp0(pref->default_val.string, *pref->varp.string)))
             return TRUE;
@@ -6203,7 +6213,8 @@ prefs_pref_to_str(pref_t *pref, pref_source_t source) {
     }
 
     case PREF_STRING:
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
         return g_strdup(*(const char **) valp);
 

epan/prefs.h

@@ -431,7 +431,7 @@ WS_DLL_PUBLIC void prefs_register_string_preference(module_t *module, const char
  * file.
  */
 WS_DLL_PUBLIC void prefs_register_filename_preference(module_t *module, const char *name,
-    const char *title, const char *description, const char **var);
+    const char *title, const char *description, const char **var, gboolean for_writing);
 
 /*
  * Register a preference with a directory name (string) value.

plugins/mate/packet-mate.c

@@ -393,7 +393,7 @@ proto_register_mate(void)
 	prefs_register_filename_preference(mate_module, "config",
 					   "Configuration Filename",
 					   "The name of the file containing the mate module's configuration",
-					   &pref_mate_config_filename);
+					   &pref_mate_config_filename, FALSE);
 
 	register_postdissector(mate_handle);
 }

ui/gtk/prefs_dlg.c

@@ -199,7 +199,8 @@ pref_show(pref_t *pref, gpointer user_data)
                                             prefs_get_string_value(pref, pref_stashed)));
     break;
 
-  case PREF_FILENAME:
+  case PREF_SAVE_FILENAME:
+  case PREF_OPEN_FILENAME:
     prefs_set_control(pref, create_preference_path_entry(main_grid, prefs_get_ordinal(pref),
                                                      label_string,
                                                      tooltip_txt,
@@ -978,7 +979,8 @@ pref_check(pref_t *pref, gpointer user_data)
     break;
 
   case PREF_STRING:
-  case PREF_FILENAME:
+  case PREF_SAVE_FILENAME:
+  case PREF_OPEN_FILENAME:
   case PREF_DIRNAME:
     /* Value can't be bad. */
     break;
@@ -1090,7 +1092,8 @@ pref_fetch(pref_t *pref, gpointer user_data)
     break;
 
   case PREF_STRING:
-  case PREF_FILENAME:
+  case PREF_SAVE_FILENAME:
+  case PREF_OPEN_FILENAME:
   case PREF_DIRNAME:
     str_val = gtk_entry_get_text(GTK_ENTRY(prefs_get_control(pref)));
     module->prefs_changed |= prefs_set_string_value(pref, str_val, pref_current);

ui/qt/module_preferences_scroll_area.cpp

@@ -189,7 +189,8 @@ pref_show(pref_t *pref, gpointer layout_ptr)
         vb->addLayout(hb);
         break;
     }
-    case PREF_FILENAME:
+    case PREF_SAVE_FILENAME:
+    case PREF_OPEN_FILENAME:
     case PREF_DIRNAME:
     {
         QLabel *label = new QLabel(prefs_get_title(pref));
@@ -260,7 +261,8 @@ ModulePreferencesScrollArea::ModulePreferencesScrollArea(module_t *module, QWidg
             connect(le, SIGNAL(textEdited(QString)), this, SLOT(uintLineEditTextEdited(QString)));
             break;
         case PREF_STRING:
-        case PREF_FILENAME:
+        case PREF_SAVE_FILENAME:
+        case PREF_OPEN_FILENAME:
         case PREF_DIRNAME:
             connect(le, SIGNAL(textEdited(QString)), this, SLOT(stringLineEditTextEdited(QString)));
             break;
@@ -308,8 +310,11 @@ ModulePreferencesScrollArea::ModulePreferencesScrollArea(module_t *module, QWidg
         case PREF_UAT:
             connect(pb, SIGNAL(pressed()), this, SLOT(uatPushButtonPressed()));
             break;
-        case PREF_FILENAME:
+        case PREF_SAVE_FILENAME:
-            connect(pb, SIGNAL(pressed()), this, SLOT(filenamePushButtonPressed()));
+            connect(pb, SIGNAL(pressed()), this, SLOT(saveFilenamePushButtonPressed()));
+            break;
+        case PREF_OPEN_FILENAME:
+            connect(pb, SIGNAL(pressed()), this, SLOT(openFilenamePushButtonPressed()));
             break;
         case PREF_DIRNAME:
             connect(pb, SIGNAL(pressed()), this, SLOT(dirnamePushButtonPressed()));
@@ -483,7 +488,7 @@ void ModulePreferencesScrollArea::uatPushButtonPressed()
     uat_dlg.exec();
 }
 
-void ModulePreferencesScrollArea::filenamePushButtonPressed()
+void ModulePreferencesScrollArea::saveFilenamePushButtonPressed()
 {
     QPushButton *filename_pb = qobject_cast<QPushButton*>(sender());
     if (!filename_pb) return;
@@ -492,8 +497,7 @@ void ModulePreferencesScrollArea::filenamePushButtonPressed()
     if (!pref) return;
 
     QString filename = QFileDialog::getSaveFileName(this, wsApp->windowTitleString(prefs_get_title(pref)),
-                                                    prefs_get_string_value(pref, pref_stashed), QString(), NULL,
+                                                    prefs_get_string_value(pref, pref_stashed));
-                                                    QFileDialog::DontConfirmOverwrite);
 
     if (!filename.isEmpty()) {
         prefs_set_string_value(pref, QDir::toNativeSeparators(filename).toStdString().c_str(), pref_stashed);
@@ -501,6 +505,22 @@ void ModulePreferencesScrollArea::filenamePushButtonPressed()
     }
 }
 
+void ModulePreferencesScrollArea::openFilenamePushButtonPressed()
+{
+    QPushButton *filename_pb = qobject_cast<QPushButton*>(sender());
+    if (!filename_pb) return;
+
+    pref_t *pref = VariantPointer<pref_t>::asPtr(filename_pb->property(pref_prop_));
+    if (!pref) return;
+
+    QString filename = QFileDialog::getOpenFileName(this, wsApp->windowTitleString(prefs_get_title(pref)),
+                                                    prefs_get_string_value(pref, pref_stashed));
+    if (!filename.isEmpty()) {
+        prefs_set_string_value(pref, QDir::toNativeSeparators(filename).toStdString().c_str(), pref_stashed);
+        updateWidgets();
+    }
+}
+
 void ModulePreferencesScrollArea::dirnamePushButtonPressed()
 {
     QPushButton *dirname_pb = qobject_cast<QPushButton*>(sender());

ui/qt/module_preferences_scroll_area.h

@@ -62,7 +62,8 @@ private slots:
     void stringLineEditTextEdited(const QString &new_str);
     void rangeSyntaxLineEditTextEdited(const QString &new_str);
     void uatPushButtonPressed();
-    void filenamePushButtonPressed();
+    void saveFilenamePushButtonPressed();
+    void openFilenamePushButtonPressed();
     void dirnamePushButtonPressed();
 };
 

ui/qt/preference_editor_frame.cpp

@@ -41,7 +41,7 @@
 #include <QPushButton>
 
 // To do:
-// - Handle PREF_FILENAME and PREF_DIRNAME.
+// - Handle PREF_SAVE_FILENAME, PREF_OPEN_FILENAME and PREF_DIRNAME.
 
 PreferenceEditorFrame::PreferenceEditorFrame(QWidget *parent) :
     AccordionFrame(parent),

ui/qt/preferences_dialog.cpp

@@ -684,18 +684,25 @@ void PreferencesDialog::on_advancedTree_itemActivated(QTreeWidgetItem *item, int
             editor = cur_line_edit_;
             break;
         }
-        case PREF_FILENAME:
+        case PREF_SAVE_FILENAME:
+        case PREF_OPEN_FILENAME:
         case PREF_DIRNAME:
         {
             QString filename;
 
-            if (prefs_get_type(pref) == PREF_FILENAME) {
+            if (prefs_get_type(pref) == PREF_SAVE_FILENAME) {
                 filename = QFileDialog::getSaveFileName(this, wsApp->windowTitleString(prefs_get_title(pref)),
                                                         prefs_get_string_value(pref, pref_stashed));
+
+            } else if (prefs_get_type(pref) == PREF_OPEN_FILENAME) {
+                filename = QFileDialog::getOpenFileName(this, wsApp->windowTitleString(prefs_get_title(pref)),
+                                                        prefs_get_string_value(pref, pref_stashed));
+
             } else {
                 filename = QFileDialog::getExistingDirectory(this, wsApp->windowTitleString(prefs_get_title(pref)),
                                                         prefs_get_string_value(pref, pref_stashed));
             }
+
             if (!filename.isEmpty()) {
                 prefs_set_string_value(pref, QDir::toNativeSeparators(filename).toStdString().c_str(), pref_stashed);
                 adv_ti->updatePref();

ui/qt/protocol_preferences_menu.cpp

@@ -39,7 +39,7 @@
 
 // To do:
 // - Elide really long items?
-// - Handle PREF_FILENAME and PREF_DIRNAME.
+// - Handle PREF_SAVE_FILENAME, PREF_OPEN_FILENAME and PREF_DIRNAME.
 // - Handle color prefs.
 
 class BoolPreferenceAction : public QAction
@@ -244,7 +244,7 @@ void ProtocolPreferencesMenu::addMenuItem(preference *pref)
     case PREF_OBSOLETE:
         break;
     default:
-        // A type we currently don't handle (e.g. PREF_FILENAME). Just open
+        // A type we currently don't handle (e.g. PREF_SAVE_FILENAME). Just open
         // the prefs dialog.
         QString title = QString("%1" UTF8_HORIZONTAL_ELLIPSIS).arg(prefs_get_title(pref));
         QAction *mpa = addAction(title);