From 0c0f731c924eadb20766e648953dfbd72c1025bc Mon Sep 17 00:00:00 2001 From: "j.novak@netsystem.cz" Date: Mon, 6 Feb 2023 18:29:25 +0000 Subject: [PATCH] extcap: Fix of handling default values --- extcap.c | 25 ++++++++++++------- extcap.h | 15 ++++++++---- ui/logray/logray_main_window_slots.cpp | 2 +- ui/qt/capture_options_dialog.cpp | 6 ++--- ui/qt/extcap_argument.cpp | 7 +----- ui/qt/extcap_argument.h | 1 - ui/qt/extcap_options_dialog.cpp | 33 +++++++++++++++++++++++--- ui/qt/interface_frame.cpp | 4 ++-- ui/qt/wireshark_main_window_slots.cpp | 2 +- 9 files changed, 64 insertions(+), 31 deletions(-) diff --git a/extcap.c b/extcap.c index 73d0ba26ac..793d484a91 100644 --- a/extcap.c +++ b/extcap.c @@ -1019,7 +1019,7 @@ extcap_get_if_configuration_values(const char * ifname, const char * argname, GH } gboolean -extcap_has_configuration(const char *ifname, gboolean is_required) +_extcap_requires_configuration_int(const char *ifname, gboolean check_required) { GList *arguments = 0; GList *walker = 0, * item = 0; @@ -1039,10 +1039,11 @@ extcap_has_configuration(const char *ifname, gboolean is_required) { extcap_arg *arg = (extcap_arg *)(item->data); /* Should required options be present, or any kind of options */ - if (!is_required) + if (!check_required) { found = TRUE; } + /* Following branch is executed when check of required items is requested */ else if (arg->is_required) { const gchar *stored = NULL; @@ -1060,13 +1061,7 @@ extcap_has_configuration(const char *ifname, gboolean is_required) if (arg->is_required) { - /* If stored and defval is identical and the argument is required, - * configuration is needed */ - if (defval && stored && g_strcmp0(stored, defval) == 0) - { - found = TRUE; - } - else if (!defval && (!stored || !*stored)) + if (!defval && (!stored || !*stored)) { found = TRUE; } @@ -1091,6 +1086,18 @@ extcap_has_configuration(const char *ifname, gboolean is_required) return found; } +gboolean +extcap_has_configuration(const char *ifname) +{ + return _extcap_requires_configuration_int(ifname, FALSE); +} + +gboolean +extcap_requires_configuration(const char *ifname) +{ + return _extcap_requires_configuration_int(ifname, TRUE); +} + static gboolean cb_verify_filter(extcap_callback_info_t cb_info) { extcap_filter_status *status = (extcap_filter_status *)cb_info.data; diff --git a/extcap.h b/extcap.h index 400d2149f9..cb74a282d3 100644 --- a/extcap.h +++ b/extcap.h @@ -185,17 +185,22 @@ extcap_free_if_configuration(GList *list, gboolean free_args); /** * Checks to see if an interface has configurable options. - * If is_required is FALSE: returns TRUE if the extcap interface has - * configurable options. - * If is_required is TRUE: returns TRUE when the extcap interface has + * Initializes the extcap interface list if that hasn't already been done. + * @param ifname Interface to check. + */ +gboolean +extcap_has_configuration(const char * ifname); + +/** + * Checks if an interface has configurable options and if all are configured. + * Returns TRUE when the extcap interface has * configurable options that required modification. (For example, when an * argument is required but empty.) * Initializes the extcap interface list if that hasn't already been done. * @param ifname Interface to check. - * @param is_required Required configuration flag. */ gboolean -extcap_has_configuration(const char * ifname, gboolean is_required); +extcap_requires_configuration(const char * ifname); /** * Checks to see if the interface has an associated toolbar. diff --git a/ui/logray/logray_main_window_slots.cpp b/ui/logray/logray_main_window_slots.cpp index abae655f02..08b426efb6 100644 --- a/ui/logray/logray_main_window_slots.cpp +++ b/ui/logray/logray_main_window_slots.cpp @@ -822,7 +822,7 @@ void LograyMainWindow::startCapture(QStringList interfaces _U_) { /* device is EXTCAP and is selected. Check if all mandatory * settings are set. */ - if (extcap_has_configuration(device->name, TRUE)) + if (extcap_requires_configuration(device->name)) { /* Request openning of extcap options dialog */ QString device_name(device->name); diff --git a/ui/qt/capture_options_dialog.cpp b/ui/qt/capture_options_dialog.cpp index 353cdcb4c4..6d4472d2d7 100644 --- a/ui/qt/capture_options_dialog.cpp +++ b/ui/qt/capture_options_dialog.cpp @@ -504,7 +504,7 @@ void CaptureOptionsDialog::itemClicked(QTreeWidgetItem *item, int column) if (device->if_info.type == IF_EXTCAP) { /* this checks if configuration is required and not yet provided or saved via prefs */ QString device_name = ti->data(col_extcap_, Qt::UserRole).value(); - if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()), FALSE)) + if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()))) { emit showExtcapOptions(device_name, false); return; @@ -538,7 +538,7 @@ void CaptureOptionsDialog::itemDoubleClicked(QTreeWidgetItem *item, int column) if (device->if_info.type == IF_EXTCAP) { /* this checks if configuration is required and not yet provided or saved via prefs */ QString device_name = ti->data(col_extcap_, Qt::UserRole).value(); - if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()), TRUE)) + if (extcap_requires_configuration((const char *)(device_name.toStdString().c_str()))) { emit showExtcapOptions(device_name, true); return; @@ -619,7 +619,7 @@ void CaptureOptionsDialog::on_buttonBox_accepted() if (device && device->if_info.type == IF_EXTCAP) { /* this checks if configuration is required and not yet provided or saved via prefs */ QString device_name = ti->data(col_extcap_, Qt::UserRole).value(); - if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()), TRUE)) + if (extcap_requires_configuration((const char *)(device_name.toStdString().c_str()))) { emit showExtcapOptions(device_name, true); return; diff --git a/ui/qt/extcap_argument.cpp b/ui/qt/extcap_argument.cpp index 16b3c12dad..0e7e6f86d3 100644 --- a/ui/qt/extcap_argument.cpp +++ b/ui/qt/extcap_argument.cpp @@ -429,11 +429,6 @@ void ExtArgRadio::setDefaultValue() ExtArgBool::ExtArgBool(extcap_arg * argument, QObject * parent) : ExtcapArgument(argument, parent), boolBox(0) {} -QWidget * ExtArgBool::createLabel(QWidget * parent) -{ - return new QWidget(parent); -} - QWidget * ExtArgBool::createEditor(QWidget * parent) { bool state = defaultBool(); @@ -486,7 +481,7 @@ QString ExtArgBool::prefValue() bool ExtArgBool::isValid() { - /* A bool is allways valid, but the base function checks on string length, + /* A bool is always valid, but the base function checks on string length, * which will fail with boolflags */ return true; } diff --git a/ui/qt/extcap_argument.h b/ui/qt/extcap_argument.h index 2efa4e42f2..9a646e56b6 100644 --- a/ui/qt/extcap_argument.h +++ b/ui/qt/extcap_argument.h @@ -235,7 +235,6 @@ class ExtArgBool : public ExtcapArgument public: ExtArgBool(extcap_arg * argument, QObject *parent = Q_NULLPTR); - virtual QWidget * createLabel(QWidget * parent); virtual QWidget * createEditor(QWidget * parent); virtual QString call(); diff --git a/ui/qt/extcap_options_dialog.cpp b/ui/qt/extcap_options_dialog.cpp index 558a3932b3..badb774476 100644 --- a/ui/qt/extcap_options_dialog.cpp +++ b/ui/qt/extcap_options_dialog.cpp @@ -448,15 +448,32 @@ bool ExtcapOptionsDialog::saveOptionToCaptureInfo() { QString call = (*iter)->call(); QString value = (*iter)->value(); + QString prefValue = (*iter)->prefValue(); if ((*iter)->argument()->arg_type != EXTCAP_ARG_BOOLFLAG && value.length() == 0) continue; - if (call.length() <= 0) + if (call.length() <= 0) { + /* BOOLFLAG was cleared, make its value empty */ + if ((*iter)->argument()->arg_type == EXTCAP_ARG_BOOLFLAG) { + *(*iter)->argument()->pref_valptr[0] = 0; + } continue; + } - if (value.compare((*iter)->defaultValue()) == 0) + if (value.compare((*iter)->defaultValue()) == 0) { + extcap_arg *arg = (*iter)->argument(); + + // If previous value is not default, set it to default value + if (arg->default_complex != NULL && arg->default_complex->_val != NULL) { + g_free(*arg->pref_valptr); + *arg->pref_valptr = g_strdup(arg->default_complex->_val); + } else { + // Set empty value if there is no default value + *arg->pref_valptr[0] = 0; + } continue; + } gchar * call_string = g_strdup(call.toStdString().c_str()); gchar * value_string = NULL; @@ -464,6 +481,14 @@ bool ExtcapOptionsDialog::saveOptionToCaptureInfo() value_string = g_strdup(value.toStdString().c_str()); g_hash_table_insert(ret_args, call_string, value_string); + + // For current value we need strdup even it is empty + value_string = g_strdup(prefValue.toStdString().c_str()); + // Update current value with new value + // We use prefValue because for bool/boolflag it returns value + // even it is false + g_free(*(*iter)->argument()->pref_valptr); + *(*iter)->argument()->pref_valptr = value_string; } if (device->external_cap_args_settings != NULL) @@ -558,7 +583,9 @@ GHashTable *ExtcapOptionsDialog::getArgumentSettings(bool useCallsAsKey, bool in if (dynamic_cast((*iter)) != NULL) { value = ((ExtArgBool *)*iter)->prefValue(); - isBoolflag = true; + // For boolflag there should be no value + if ((*iter)->argument()->arg_type != EXTCAP_ARG_BOOLFLAG) + isBoolflag = true; } else if (dynamic_cast((*iter)) != NULL) { diff --git a/ui/qt/interface_frame.cpp b/ui/qt/interface_frame.cpp index 7aa6b7fb96..70937ff5b7 100644 --- a/ui/qt/interface_frame.cpp +++ b/ui/qt/interface_frame.cpp @@ -426,7 +426,7 @@ void InterfaceFrame::on_interfaceTree_doubleClicked(const QModelIndex &index) if (extcap_string.length() > 0) { /* this checks if configuration is required and not yet provided or saved via prefs */ - if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()), TRUE)) + if (extcap_requires_configuration((const char *)(device_name.toStdString().c_str()))) { emit showExtcapOptions(device_name, true); return; @@ -458,7 +458,7 @@ void InterfaceFrame::on_interfaceTree_clicked(const QModelIndex &index) if (extcap_string.length() > 0) { /* this checks if configuration is required and not yet provided or saved via prefs */ - if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()), FALSE)) + if (extcap_has_configuration((const char *)(device_name.toStdString().c_str()))) { emit showExtcapOptions(device_name, false); return; diff --git a/ui/qt/wireshark_main_window_slots.cpp b/ui/qt/wireshark_main_window_slots.cpp index e6051a359b..c29807f0fe 100644 --- a/ui/qt/wireshark_main_window_slots.cpp +++ b/ui/qt/wireshark_main_window_slots.cpp @@ -871,7 +871,7 @@ void WiresharkMainWindow::startCapture(QStringList interfaces _U_) { /* device is EXTCAP and is selected. Check if all mandatory * settings are set. */ - if (extcap_has_configuration(device->name, TRUE)) + if (extcap_requires_configuration(device->name)) { /* Request openning of extcap options dialog */ QString device_name(device->name);