[PATCH 2/3] global parser functions return 1 when property unchanged

Matthew Wang matthewmwang at chromium.org
Fri Jun 26 18:57:07 EDT 2020


Currently, wpa_config_set, the function that sets per-network
properties, returns 1 when a property it attempts to set is unchanged.
Its global parallel, wpa_config_process_global, doesn't do this even
though much of the code is very similar. Change this, and several of the
parser functions, to resemble the per-network parser and setter
functions.

Signed-off-by: Matthew Wang <matthewmwang at chromium.org>
---
 src/utils/wpabuf.h                      | 15 ++++++
 wpa_supplicant/config.c                 | 63 +++++++++++++++++++++++--
 wpa_supplicant/ctrl_iface.c             |  2 +
 wpa_supplicant/dbus/dbus_new_handlers.c |  8 ++--
 4 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/src/utils/wpabuf.h b/src/utils/wpabuf.h
index 01da41b32..ecdc08f84 100644
--- a/src/utils/wpabuf.h
+++ b/src/utils/wpabuf.h
@@ -70,6 +70,21 @@ static inline size_t wpabuf_tailroom(const struct wpabuf *buf)
 	return buf->size - buf->used;
 }
 
+/**
+ * wpabuf_cmp - Check if two buffers contain the same data
+ * @a: wpabuf buffer
+ * @b: wpabuf buffer
+ * Returns: 0 if the two buffers contain the same data and non-zero otherwise
+ */
+static inline int wpabuf_cmp(const struct wpabuf *a, const struct wpabuf *b)
+{
+	if (a == NULL && b == NULL)
+		return 0;
+	if (a && b && wpabuf_size(a) == wpabuf_size(b))
+		return os_memcmp(a->buf, b->buf, wpabuf_size(a));
+	return -1;
+}
+
 /**
  * wpabuf_head - Get pointer to the head of the buffer data
  * @buf: wpabuf buffer
diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
index 0b4a66ad7..e4fa43ec7 100644
--- a/wpa_supplicant/config.c
+++ b/wpa_supplicant/config.c
@@ -4360,13 +4360,31 @@ void wpa_config_debug_dump_networks(struct wpa_config *config)
 #endif /* CONFIG_NO_STDOUT_DEBUG */
 
 
+/**
+ * Structure for global configuration parsing. This data is used to implement a
+ * generic parser for the global interface configuration. The table of variables
+ * is defined below in this file (global_fields[]).
+ */
 struct global_parse_data {
+	/* Configuration variable name */
 	char *name;
+
+	/* Parser function for this variable. The parser functions return 0 or 1
+	 * to indicate success. Value 0 indicates that the parameter value may
+	 * have change while value 1 means that the value did not change.
+	 * Error cases (failure to parse the string) are indicated by returning
+	 * -1. */
 	int (*parser)(const struct global_parse_data *data,
 		      struct wpa_config *config, int line, const char *value);
+
+	/* Getter function to print the variable in text format to buf. */
 	int (*get)(const char *name, struct wpa_config *config, long offset,
 		   char *buf, size_t buflen, int pretty_print);
+
+	/* Variable specific parameters for the parser. */
 	void *param1, *param2, *param3;
+
+	/* Indicates which configuration variable has changed. */
 	unsigned int changed_flag;
 };
 
@@ -4385,8 +4403,10 @@ static int wpa_global_config_parse_int(const struct global_parse_data *data,
 			   line, pos);
 		return -1;
 	}
-	*dst = val;
 
+	if (*dst == val)
+		return 1;
+	*dst = val;
 	wpa_printf(MSG_DEBUG, "%s=%d", data->name, *dst);
 
 	if (data->param2 && *dst < (long) data->param2) {
@@ -4413,7 +4433,7 @@ static int wpa_global_config_parse_str(const struct global_parse_data *data,
 				       struct wpa_config *config, int line,
 				       const char *pos)
 {
-	size_t len;
+	size_t len, prev_len;
 	char **dst, *tmp;
 
 	len = os_strlen(pos);
@@ -4437,11 +4457,22 @@ static int wpa_global_config_parse_str(const struct global_parse_data *data,
 		return -1;
 	}
 
+	dst = (char **) (((u8 *) config) + (long) data->param1);
+	if (*dst)
+		prev_len = os_strlen(*dst);
+	else
+		prev_len = 0;
+
+	/* No change to the previously configured value */
+	if ((*dst == NULL && pos == NULL) ||
+	    (*dst && pos && prev_len == len &&
+	     os_memcmp(*dst, pos, len) == 0))
+		return 1;
+
 	tmp = os_strdup(pos);
 	if (tmp == NULL)
 		return -1;
 
-	dst = (char **) (((u8 *) config) + (long) data->param1);
 	os_free(*dst);
 	*dst = tmp;
 	wpa_printf(MSG_DEBUG, "%s='%s'", data->name, *dst);
@@ -4482,6 +4513,10 @@ static int wpa_global_config_parse_bin(const struct global_parse_data *data,
 		return -1;
 
 	dst = (struct wpabuf **) (((u8 *) config) + (long) data->param1);
+	if (wpabuf_cmp(*dst, tmp) == 0) {
+		wpabuf_free(tmp);
+		return 1;
+	}
 	wpabuf_free(*dst);
 	*dst = tmp;
 	wpa_printf(MSG_DEBUG, "%s", data->name);
@@ -4523,6 +4558,8 @@ static int wpa_global_config_parse_ipv4(const struct global_parse_data *data,
 		return -1;
 
 	dst = (u32 *) (((u8 *) config) + (long) data->param1);
+	if (os_memcmp(dst, &addr.u.v4.s_addr, 4) == 0)
+		return 1;
 	os_memcpy(dst, &addr.u.v4.s_addr, 4);
 	wpa_printf(MSG_DEBUG, "%s = 0x%x", data->name,
 		   WPA_GET_BE32((u8 *) dst));
@@ -4540,6 +4577,8 @@ static int wpa_config_process_country(const struct global_parse_data *data,
 		wpa_printf(MSG_DEBUG, "Invalid country set");
 		return -1;
 	}
+	if (pos[0] == config->country[0] && pos[1] == config->country[1])
+		return 1;
 	config->country[0] = pos[0];
 	config->country[1] = pos[1];
 	wpa_printf(MSG_DEBUG, "country='%c%c'",
@@ -5158,6 +5197,19 @@ const char * wpa_config_get_global_field_name(unsigned int i, int *no_var)
 }
 
 
+/**
+ * wpa_config_process_global - Set a variable in global configuration
+ * @config: Pointer to global configuration data
+ * @pos: Name and value in the format "{name}={value}"
+ * @line: Line number in configuration file or 0 if not used
+ * Returns: 0 on success with possible change in value, 1 on success with no
+ * change to previously configured value, or -1 on failure
+ *
+ * This function can be used to set global configuration variables based on
+ * both the configuration file and management interface input. The value
+ * parameter must be in the same format as the text-based configuration file is
+ * using. For example, strings are using double quotation marks.
+ */
 int wpa_config_process_global(struct wpa_config *config, char *pos, int line)
 {
 	size_t i;
@@ -5170,11 +5222,14 @@ int wpa_config_process_global(struct wpa_config *config, char *pos, int line)
 		    pos[flen] != '=')
 			continue;
 
-		if (field->parser(field, config, line, pos + flen + 1)) {
+		ret = field->parser(field, config, line, pos + flen + 1);
+		if (ret < 0) {
 			wpa_printf(MSG_ERROR, "Line %d: failed to "
 				   "parse '%s'.", line, pos);
 			ret = -1;
 		}
+		if (ret == 1)
+			break;
 		if (field->changed_flag == CFG_CHANGED_NFC_PASSWORD_TOKEN)
 			config->wps_nfc_pw_from_config = 1;
 		config->changed_parameters |= field->changed_flag;
diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
index 8d2ccbae5..eea8b40f4 100644
--- a/wpa_supplicant/ctrl_iface.c
+++ b/wpa_supplicant/ctrl_iface.c
@@ -884,6 +884,8 @@ static int wpa_supplicant_ctrl_iface_set(struct wpa_supplicant *wpa_s,
 		ret = wpa_config_process_global(wpa_s->conf, cmd, -1);
 		if (ret == 0)
 			wpa_supplicant_update_config(wpa_s);
+		else if (ret == 1)
+			ret = 0;
 	}
 
 	return ret;
diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
index 93eeb30e1..18b66e303 100644
--- a/wpa_supplicant/dbus/dbus_new_handlers.c
+++ b/wpa_supplicant/dbus/dbus_new_handlers.c
@@ -3948,14 +3948,14 @@ dbus_bool_t wpas_dbus_setter_iface_global(
 		return FALSE;
 	}
 
-	if (wpa_config_process_global(wpa_s->conf, buf, -1)) {
+	ret = wpa_config_process_global(wpa_s->conf, buf, -1);
+	if (ret < 0) {
 		dbus_set_error(error, DBUS_ERROR_INVALID_ARGS,
 			       "Failed to set interface property %s",
 			       property_desc->dbus_property);
 		return FALSE;
-	}
-
-	wpa_supplicant_update_config(wpa_s);
+	} else if (ret == 0)
+		wpa_supplicant_update_config(wpa_s);
 	return TRUE;
 }
 
-- 
2.27.0.212.ge8ba1cc988-goog




More information about the Hostap mailing list