[PATCH 1/3] wsc: Split out enrollee state machine to own object

[PATCH 1/3] wsc: Split out enrollee state machine to own object

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 23073 bytes --]

Create struct wsc_enrollee which is allocated with wsc_enrollee_new,
taking a done callback as a parameter.  The callback is always
called so there's no need for a separate destroy callback.  The object
only lives until the done callback happens so wsc_enrollee_cancel/destroy
can only be used before this.

Looks like the rest of the file is simplified thanks to this.
---
 src/wsc.c | 504 +++++++++++++++++++++++++++++-------------------------
 src/wsc.h |   9 +
 2 files changed, 276 insertions(+), 237 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index d50f240f..e381b84f 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -51,9 +51,20 @@
 
 static uint32_t netdev_watch = 0;
 
+struct wsc_enrollee {
+	struct netdev *netdev;
+	struct wsc_credentials_info creds[3];
+	uint32_t n_creds;
+	struct l_settings *eap_settings;
+	wsc_done_cb_t done_cb;
+	void *done_data;
+	bool disconnecting : 1;
+};
+
 struct wsc {
 	struct netdev *netdev;
 	struct station *station;
+	struct wsc_enrollee *enrollee;
 	struct l_dbus_message *pending;
 	struct l_dbus_message *pending_cancel;
 	uint8_t *wsc_ies;
@@ -62,16 +73,6 @@ struct wsc {
 	uint32_t scan_id;
 	struct scan_bss *target;
 	uint32_t station_state_watch;
-	struct wsc_credentials_info creds[3];
-	uint32_t n_creds;
-	struct l_settings *eap_settings;
-	char *pin;
-
-	wsc_done_cb_t done_cb;
-	void *done_data;
-	uint16_t config_method;
-
-	bool wsc_association : 1;
 };
 
 static struct l_dbus_message *wsc_error_session_overlap(
@@ -113,171 +114,50 @@ static struct l_dbus_message *wsc_error_time_expired(struct l_dbus_message *msg)
 					"No APs in PIN mode found in "
 					"the alloted time");
 }
-static void wsc_try_credentials(struct wsc *wsc,
-					struct wsc_credentials_info *creds,
-					unsigned int n_creds)
-{
-	unsigned int i;
-	struct network *network;
-	struct scan_bss *bss;
-
-	for (i = 0; i < n_creds; i++) {
-		network = station_network_find(wsc->station, creds[i].ssid,
-						creds[i].security);
-		if (!network)
-			continue;
-
-		bss = network_bss_find_by_addr(network, creds[i].addr);
-
-		if (!bss)
-			bss = network_bss_select(network, true);
-
-		if (!bss)
-			continue;
-
-		if (creds[i].security == SECURITY_PSK) {
-			bool ret;
-
-			/*
-			 * Prefer setting passphrase, this will work for both
-			 * WPA2 and WPA3 since the PSK can always be generated
-			 * if needed
-			 */
-			if (creds[i].has_passphrase)
-				ret = network_set_passphrase(network,
-							creds[i].passphrase);
-			else
-				ret = network_set_psk(network, creds[i].psk);
-
-			if (!ret)
-				continue;
-		}
-
-		station_connect_network(wsc->station, network, bss,
-								wsc->pending);
-		l_dbus_message_unref(wsc->pending);
-		wsc->pending = NULL;
-
-		return;
-	}
-
-	dbus_pending_reply(&wsc->pending,
-					wsc_error_not_reachable(wsc->pending));
-	station_set_autoconnect(wsc->station, true);
-}
 
-static void wsc_store_credentials(struct wsc_credentials_info *creds,
-					unsigned int n_creds)
+static void wsc_enrollee_free(struct wsc_enrollee *wsce)
 {
-	unsigned int i;
-
-	for (i = 0; i < n_creds; i++) {
-		enum security security = creds[i].security;
-		const char *ssid = creds[i].ssid;
-		struct l_settings *settings = l_settings_new();
-
-		l_debug("Storing credential for '%s(%s)'", ssid,
-						security_to_str(security));
-
-		if (security == SECURITY_PSK) {
-			char *hex = l_util_hexstring(creds[i].psk,
-						sizeof(creds[i].psk));
-
-			l_settings_set_value(settings, "Security",
-							"PreSharedKey", hex);
-			explicit_bzero(hex, strlen(hex));
-			l_free(hex);
-		}
-
-		storage_network_sync(security, ssid, settings);
-		l_settings_free(settings);
-
-		/*
-		 * TODO: Mark this network as known.  We might be getting
-		 * multiple credentials from WSC, so there is a possibility
-		 * that the network is not known and / or not in scan results.
-		 * In both cases, the network should be considered for
-		 * auto-connect.  Note, since we sync the settings, the next
-		 * reboot will put the network on the known list.
-		 */
-	}
+	l_settings_free(wsce->eap_settings);
+	explicit_bzero(wsce->creds, sizeof(wsce->creds));
+	l_free(wsce);
 }
 
-static void wsc_disconnect_cb(struct netdev *netdev, bool success,
-							void *user_data)
-{
-	struct wsc *wsc = user_data;
-	struct l_dbus_message *reply;
-
-	l_debug("%p, success: %d", wsc, success);
-
-	wsc->wsc_association = false;
-
-	reply = l_dbus_message_new_method_return(wsc->pending_cancel);
-	l_dbus_message_set_arguments(reply, "");
-	dbus_pending_reply(&wsc->pending_cancel, reply);
-
-	station_set_autoconnect(wsc->station, true);
-}
-
-static void wsc_connect_cleanup(struct wsc *wsc)
-{
-	wsc->wsc_association = false;
-
-	l_settings_free(wsc->eap_settings);
-	wsc->eap_settings = NULL;
-
-	if (wsc->pin) {
-		explicit_bzero(wsc->pin, strlen(wsc->pin));
-		l_free(wsc->pin);
-		wsc->pin = NULL;
-	}
-
-	wsc->target = NULL;
-}
-
-static void wsc_connect_cb(struct netdev *netdev, enum netdev_result result,
+static void wsc_enrollee_connect_cb(struct netdev *netdev,
+					enum netdev_result result,
 					void *event_data, void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_enrollee *wsce = user_data;
 
-	l_debug("%d, result: %d", netdev_get_ifindex(wsc->netdev), result);
+	l_debug("%d, result: %d", netdev_get_ifindex(wsce->netdev), result);
 
-	wsc_connect_cleanup(wsc);
+	if (wsce->disconnecting)
+		return;	/* Free the state in the disconnect callback */
 
-	if (result == NETDEV_RESULT_HANDSHAKE_FAILED && wsc->n_creds > 0) {
-		struct wsc_credentials_info creds[L_ARRAY_SIZE(wsc->creds)];
-		uint32_t n_creds = wsc->n_creds;
+	if (!wsce->done_cb)
+		goto done;
 
-		/*
-		 * Once we call done_cb, the state is assumed to be wiped,
-		 * so use a temporary array for creds here
-		 */
-		memcpy(creds, wsc->creds, sizeof(creds));
-		explicit_bzero(wsc->creds, sizeof(creds));
-		wsc->n_creds = 0;
-		wsc->done_cb(0, creds, n_creds, wsc->done_data);
-		explicit_bzero(creds, sizeof(creds));
-		return;
+	if (result == NETDEV_RESULT_HANDSHAKE_FAILED && wsce->n_creds > 0) {
+		wsce->done_cb(0, wsce->creds, wsce->n_creds, wsce->done_data);
+		goto done;
 	}
 
 	switch (result) {
 	case NETDEV_RESULT_ABORTED:
-		wsc->done_cb(-ECANCELED, NULL, 0, wsc->done_data);
-		return;
+		wsce->done_cb(-ECANCELED, NULL, 0, wsce->done_data);
+		break;
 	case NETDEV_RESULT_HANDSHAKE_FAILED:
-		wsc->done_cb(-ENOKEY, NULL, 0, wsc->done_data);
+		wsce->done_cb(-ENOKEY, NULL, 0, wsce->done_data);
 		break;
 	default:
-		wsc->done_cb(-EIO, NULL, 0, wsc->done_data);
+		wsce->done_cb(-EIO, NULL, 0, wsce->done_data);
 		break;
 	}
 
-	if (wsc->station)
-		station_set_autoconnect(wsc->station, true);
+done:
+	wsc_enrollee_free(wsce);
 }
 
-static void wsc_credential_obtained(struct wsc *wsc,
+static void wsc_enrollee_credential_obtained(struct wsc_enrollee *wsce,
 					const struct wsc_credential *cred)
 {
 	uint16_t auth_mask;
@@ -294,7 +174,7 @@ static void wsc_credential_obtained(struct wsc *wsc,
 		l_debug("Key (%u): %.*s", cred->network_key_len,
 				cred->network_key_len, cred->network_key);
 
-	if (wsc->n_creds == L_ARRAY_SIZE(wsc->creds)) {
+	if (wsce->n_creds == L_ARRAY_SIZE(wsce->creds)) {
 		l_warn("Maximum number of credentials obtained, ignoring...");
 		return;
 	}
@@ -304,8 +184,8 @@ static void wsc_credential_obtained(struct wsc *wsc,
 		return;
 	}
 
-	memcpy(wsc->creds[wsc->n_creds].ssid, cred->ssid, cred->ssid_len);
-	wsc->creds[wsc->n_creds].ssid[cred->ssid_len] = '\0';
+	memcpy(wsce->creds[wsce->n_creds].ssid, cred->ssid, cred->ssid_len);
+	wsce->creds[wsce->n_creds].ssid[cred->ssid_len] = '\0';
 
 	/* We only support open/personal wpa/personal wpa2 */
 	auth_mask = WSC_AUTHENTICATION_TYPE_OPEN |
@@ -324,11 +204,11 @@ static void wsc_credential_obtained(struct wsc *wsc,
 			return;
 		}
 
-		wsc->creds[wsc->n_creds].security = SECURITY_NONE;
+		wsce->creds[wsce->n_creds].security = SECURITY_NONE;
 	} else
-		wsc->creds[wsc->n_creds].security = SECURITY_PSK;
+		wsce->creds[wsce->n_creds].security = SECURITY_PSK;
 
-	switch (wsc->creds[wsc->n_creds].security) {
+	switch (wsce->creds[wsce->n_creds].security) {
 	case SECURITY_NONE:
 		if (cred->network_key_len != 0) {
 			l_warn("ignoring invalid open key length");
@@ -347,14 +227,14 @@ static void wsc_credential_obtained(struct wsc *wsc,
 				return;
 			}
 
-			memcpy(wsc->creds[wsc->n_creds].psk, decoded, 32);
+			memcpy(wsce->creds[wsce->n_creds].psk, decoded, 32);
 			explicit_bzero(decoded, 32);
 			l_free(decoded);
 		} else {
-			strncpy(wsc->creds[wsc->n_creds].passphrase,
+			strncpy(wsce->creds[wsce->n_creds].passphrase,
 					(const char *) cred->network_key,
 					cred->network_key_len);
-			wsc->creds[wsc->n_creds].has_passphrase = true;
+			wsce->creds[wsce->n_creds].has_passphrase = true;
 		}
 
 		break;
@@ -362,25 +242,27 @@ static void wsc_credential_obtained(struct wsc *wsc,
 		return;
 	}
 
-	for (i = 0; i < wsc->n_creds; i++) {
-		if (strcmp(wsc->creds[i].ssid, wsc->creds[wsc->n_creds].ssid))
+	for (i = 0; i < wsce->n_creds; i++) {
+		if (strcmp(wsce->creds[i].ssid,
+				wsce->creds[wsce->n_creds].ssid))
 			continue;
 
 		l_warn("Found duplicate credentials for SSID: %s",
-				wsc->creds[i].ssid);
-		explicit_bzero(&wsc->creds[wsc->n_creds],
-				sizeof(wsc->creds[wsc->n_creds]));
+				wsce->creds[i].ssid);
+		explicit_bzero(&wsce->creds[wsce->n_creds],
+				sizeof(wsce->creds[wsce->n_creds]));
 		return;
 	}
 
-	memcpy(wsc->creds[wsc->n_creds].addr, cred->addr, 6);
-	wsc->n_creds += 1;
+	memcpy(wsce->creds[wsce->n_creds].addr, cred->addr, 6);
+	wsce->n_creds += 1;
 }
 
-static void wsc_netdev_event(struct netdev *netdev, enum netdev_event event,
+static void wsc_enrollee_netdev_event(struct netdev *netdev,
+					enum netdev_event event,
 					void *event_data, void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_enrollee *wsce = user_data;
 
 	switch (event) {
 	case NETDEV_EVENT_AUTHENTICATING:
@@ -391,8 +273,9 @@ static void wsc_netdev_event(struct netdev *netdev, enum netdev_event event,
 		break;
 	case NETDEV_EVENT_DISCONNECT_BY_AP:
 		l_debug("Disconnect by AP");
-		wsc_connect_cb(wsc->netdev, NETDEV_RESULT_HANDSHAKE_FAILED,
-				event_data, wsc);
+		wsc_enrollee_connect_cb(wsce->netdev,
+					NETDEV_RESULT_HANDSHAKE_FAILED,
+					event_data, wsce);
 		break;
 	case NETDEV_EVENT_RSSI_THRESHOLD_LOW:
 	case NETDEV_EVENT_RSSI_THRESHOLD_HIGH:
@@ -403,11 +286,11 @@ static void wsc_netdev_event(struct netdev *netdev, enum netdev_event event,
 	};
 }
 
-static void wsc_handshake_event(struct handshake_state *hs,
-				enum handshake_event event, void *user_data,
-				...)
+static void wsc_enrollee_handshake_event(struct handshake_state *hs,
+						enum handshake_event event,
+						void *user_data, ...)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_enrollee *wsce = user_data;
 	va_list args;
 
 	va_start(args, user_data);
@@ -422,7 +305,7 @@ static void wsc_handshake_event(struct handshake_state *hs,
 
 		switch (eap_event) {
 		case EAP_WSC_EVENT_CREDENTIAL_OBTAINED:
-			wsc_credential_obtained(wsc,
+			wsc_enrollee_credential_obtained(wsce,
 				va_arg(args, const struct wsc_credential *));
 			break;
 		default:
@@ -454,20 +337,18 @@ static inline enum wsc_rf_band freq_to_rf_band(uint32_t freq)
 	return WSC_RF_BAND_2_4_GHZ;
 }
 
-static void wsc_connect(struct wsc *wsc)
+static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
+					const char *pin)
 {
 	struct handshake_state *hs;
 	struct l_settings *settings = l_settings_new();
-	struct scan_bss *bss = wsc->target;
 	int r;
 	struct wsc_association_request request;
 	uint8_t *pdu;
 	size_t pdu_len;
 	struct iovec ie_iov;
 
-	wsc->target = NULL;
-
-	hs = netdev_handshake_state_new(wsc->netdev);
+	hs = netdev_handshake_state_new(wsce->netdev);
 
 	l_settings_set_string(settings, "Security", "EAP-Identity",
 					"WFA-SimpleConfig-Enrollee-1-0");
@@ -482,25 +363,23 @@ static void wsc_connect(struct wsc *wsc)
 	l_settings_set_string(settings, "WSC", "PrimaryDeviceType",
 					"0-00000000-0");
 	l_settings_set_string(settings, "WSC", "EnrolleeMAC",
-		util_address_to_string(netdev_get_address(wsc->netdev)));
+		util_address_to_string(netdev_get_address(wsce->netdev)));
 
-	if (wsc->config_method == WSC_CONFIGURATION_METHOD_KEYPAD) {
+	if (pin) {
 		enum wsc_device_password_id dpid;
 
-		if (strlen(wsc->pin) == 4 ||
-				wsc_pin_is_checksum_valid(wsc->pin))
+		if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
 			dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
 		else
 			dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
 
 		l_settings_set_uint(settings, "WSC", "DevicePasswordId", dpid);
-		l_settings_set_string(settings, "WSC", "DevicePassword",
-					wsc->pin);
+		l_settings_set_string(settings, "WSC", "DevicePassword", pin);
 	}
 
-	handshake_state_set_event_func(hs, wsc_handshake_event, wsc);
+	handshake_state_set_event_func(hs, wsc_enrollee_handshake_event, wsce);
 	handshake_state_set_8021x_config(hs, settings);
-	wsc->eap_settings = settings;
+	wsce->eap_settings = settings;
 
 	request.version2 = true;
 	request.request_type = WSC_REQUEST_TYPE_ENROLLEE_OPEN_8021X;
@@ -520,19 +399,164 @@ static void wsc_connect(struct wsc *wsc)
 		goto error;
 	}
 
-	r = netdev_connect(wsc->netdev, bss, hs, &ie_iov, 1, wsc_netdev_event,
-				wsc_connect_cb, wsc);
+	r = netdev_connect(wsce->netdev, bss, hs, &ie_iov, 1,
+				wsc_enrollee_netdev_event,
+				wsc_enrollee_connect_cb, wsce);
 	l_free(ie_iov.iov_base);
 
-	if (r < 0)
-		goto error;
+	if (r == 0)
+		return 0;
 
-	wsc->wsc_association = true;
-	return;
 error:
 	handshake_state_free(hs);
-	wsc_connect_cleanup(wsc);
-	wsc->done_cb(r, NULL, 0, wsc->done_data);
+	return r;
+}
+
+struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
+					struct scan_bss *target,
+					const char *pin,
+					wsc_done_cb_t done_cb, void *user_data)
+{
+	struct wsc_enrollee *wsce;
+
+	wsce = l_new(struct wsc_enrollee, 1);
+	wsce->netdev = netdev;
+	wsce->done_cb = done_cb;
+	wsce->done_data = user_data;
+
+	if (wsc_enrollee_connect(wsce, target, pin) == 0)
+		return wsce;
+
+	wsc_enrollee_free(wsce);
+	return NULL;
+}
+
+static void wsc_enrollee_disconnect_cb(struct netdev *netdev, bool result,
+					void *user_data)
+{
+	struct wsc_enrollee *wsce = user_data;
+
+	wsce->done_cb(-ECANCELED, NULL, 0, wsce->done_data);
+	wsc_enrollee_free(wsce);
+}
+
+void wsc_enrollee_cancel(struct wsc_enrollee *wsce, bool defer_cb)
+{
+	if (defer_cb) {
+		wsce->disconnecting = true;
+		netdev_disconnect(wsce->netdev, wsc_enrollee_disconnect_cb,
+					wsce);
+	} else {
+		wsce->done_cb(-ECANCELED, NULL, 0, wsce->done_data);
+		wsce->done_cb = NULL;
+		/*
+		 * Results in a call to
+		 * wsc_enrollee_connect_cb -> wsc_enrollee_free
+		 */
+		netdev_disconnect(wsce->netdev, NULL, NULL);
+	}
+}
+
+/* To be called instead of wsc_enrollee_cancel if we know the netdev is gone */
+void wsc_enrollee_destroy(struct wsc_enrollee *wsce)
+{
+	/*
+	 * Note: wsc_enrollee_cancel(wsce, true) may have been called before
+	 * but since the netdev is going away wsc_enrollee_disconnect_cb
+	 * is not happening anymore.
+	 */
+	wsce->done_cb(-ECANCELED, NULL, 0, wsce->done_data);
+	wsc_enrollee_free(wsce);
+}
+
+static void wsc_try_credentials(struct wsc *wsc,
+				struct wsc_credentials_info *creds,
+				unsigned int n_creds)
+{
+	unsigned int i;
+	struct network *network;
+	struct scan_bss *bss;
+
+	for (i = 0; i < n_creds; i++) {
+		network = station_network_find(wsc->station, creds[i].ssid,
+						creds[i].security);
+		if (!network)
+			continue;
+
+		bss = network_bss_find_by_addr(network, creds[i].addr);
+
+		if (!bss)
+			bss = network_bss_select(network, true);
+
+		if (!bss)
+			continue;
+
+		if (creds[i].security == SECURITY_PSK) {
+			bool ret;
+
+			/*
+			 * Prefer setting passphrase, this will work for both
+			 * WPA2 and WPA3 since the PSK can always be generated
+			 * if needed
+			 */
+			if (creds[i].has_passphrase)
+				ret = network_set_passphrase(network,
+							creds[i].passphrase);
+			else
+				ret = network_set_psk(network, creds[i].psk);
+
+			if (!ret)
+				continue;
+		}
+
+		station_connect_network(wsc->station, network, bss,
+								wsc->pending);
+		l_dbus_message_unref(wsc->pending);
+		wsc->pending = NULL;
+
+		return;
+	}
+
+	dbus_pending_reply(&wsc->pending,
+					wsc_error_not_reachable(wsc->pending));
+	station_set_autoconnect(wsc->station, true);
+}
+
+static void wsc_store_credentials(struct wsc_credentials_info *creds,
+					unsigned int n_creds)
+{
+	unsigned int i;
+
+	for (i = 0; i < n_creds; i++) {
+		enum security security = creds[i].security;
+		const char *ssid = creds[i].ssid;
+		struct l_settings *settings = l_settings_new();
+
+		l_debug("Storing credential for '%s(%s)'", ssid,
+						security_to_str(security));
+
+		if (security == SECURITY_PSK) {
+			char *hex = l_util_hexstring(creds[i].psk,
+						sizeof(creds[i].psk));
+
+			l_settings_set_value(settings, "Security",
+							"PreSharedKey", hex);
+			explicit_bzero(hex, strlen(hex));
+			l_free(hex);
+		}
+
+		storage_network_sync(security, ssid, settings);
+		l_settings_free(settings);
+
+		/*
+		 * TODO: Mark this network as known.  We might be getting
+		 * multiple credentials from WSC, so there is a possibility
+		 * that the network is not known and / or not in scan results.
+		 * In both cases, the network should be considered for
+		 * auto-connect.  Note, since we sync the settings, the next
+		 * reboot will put the network on the known list.
+		 */
+	}
 }
 
 /* Done callback for when WSC is triggered by DBus methods */
@@ -541,14 +565,28 @@ static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
 {
 	struct wsc *wsc = user_data;
 
+	wsc->enrollee = NULL;
+	wsc->target = NULL;
+
 	l_debug("err=%i", err);
 
+	if (err && wsc->station)
+		station_set_autoconnect(wsc->station, true);
+
 	switch (err) {
 	case 0:
 		break;
 	case -ECANCELED:
-		dbus_pending_reply(&wsc->pending,
+		/* Send reply if we haven't already sent one e.g. in Cancel() */
+		if (wsc->pending)
+			dbus_pending_reply(&wsc->pending,
 					dbus_error_aborted(wsc->pending));
+
+		if (wsc->pending_cancel)
+			dbus_pending_reply(&wsc->pending_cancel,
+					l_dbus_message_new_method_return(
+							wsc->pending_cancel));
+
 		return;
 	case -ENOKEY:
 		dbus_pending_reply(&wsc->pending,
@@ -568,6 +606,21 @@ static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
 	wsc_try_credentials(wsc, creds, n_creds);
 }
 
+static void wsc_connect(struct wsc *wsc)
+{
+	const char *pin = NULL;
+
+	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin"))
+		l_dbus_message_get_arguments(wsc->pending, "s", &pin);
+
+	wsc->enrollee = wsc_enrollee_new(wsc->netdev, wsc->target, pin,
+						wsc_dbus_done_cb, wsc);
+	if (wsc->enrollee)
+		return;
+
+	wsc_dbus_done_cb(-EIO, NULL, 0, wsc);
+}
+
 static void station_state_watch(enum station_state state, void *userdata)
 {
 	struct wsc *wsc = userdata;
@@ -594,22 +647,6 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
 	wsc->target = target;
 	station_set_autoconnect(wsc->station, false);
 
-	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin")) {
-		char *pin;
-
-		wsc->config_method = WSC_CONFIGURATION_METHOD_KEYPAD;
-
-		if (!l_dbus_message_get_arguments(wsc->pending, "s", &pin))
-			goto error;
-
-		wsc->pin = l_strdup(pin);
-	} else
-		wsc->config_method =
-			WSC_CONFIGURATION_METHOD_VIRTUAL_PUSH_BUTTON;
-
-	wsc->done_cb = wsc_dbus_done_cb;
-	wsc->done_data = wsc;
-
 	switch (station_get_state(wsc->station)) {
 	case STATION_STATE_DISCONNECTED:
 		wsc_connect(wsc);
@@ -633,7 +670,7 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
 		break;
 	}
 error:
-	wsc_connect_cleanup(wsc);
+	wsc->target = NULL;
 	dbus_pending_reply(&wsc->pending, dbus_error_failed(wsc->pending));
 }
 
@@ -1089,13 +1126,15 @@ static struct l_dbus_message *wsc_cancel(struct l_dbus *dbus,
 						void *user_data)
 {
 	struct wsc *wsc = user_data;
-	struct l_dbus_message *reply;
 
 	l_debug("");
 
 	if (!wsc->pending)
 		return dbus_error_not_available(message);
 
+	if (wsc->pending_cancel)
+		return dbus_error_busy(message);
+
 	wsc_cancel_scan(wsc);
 
 	if (wsc->station_state_watch) {
@@ -1105,25 +1144,14 @@ static struct l_dbus_message *wsc_cancel(struct l_dbus *dbus,
 		wsc->target = NULL;
 	}
 
-	if (wsc->wsc_association) {
-		int r;
-
-		r = netdev_disconnect(wsc->netdev, wsc_disconnect_cb, wsc);
-		if (r == 0) {
-			wsc->pending_cancel = l_dbus_message_ref(message);
-			return NULL;
-		}
-
-		l_warn("Unable to initiate disconnect: %s", strerror(-r));
-		wsc->wsc_association = false;
-	}
-
 	dbus_pending_reply(&wsc->pending, dbus_error_aborted(wsc->pending));
 
-	reply = l_dbus_message_new_method_return(message);
-	l_dbus_message_set_arguments(reply, "");
-
-	return reply;
+	if (wsc->enrollee) {
+		wsc_enrollee_cancel(wsc->enrollee, true);
+		wsc->pending_cancel = l_dbus_message_ref(message);
+		return NULL;
+	} else
+		return l_dbus_message_new_method_return(message);
 }
 
 static void setup_wsc_interface(struct l_dbus_interface *interface)
@@ -1143,7 +1171,6 @@ static void wsc_free(void *userdata)
 	struct wsc *wsc = userdata;
 
 	wsc_cancel_scan(wsc);
-	wsc_connect_cleanup(wsc);
 
 	if (wsc->station_state_watch) {
 		station_remove_state_watch(wsc->station,
@@ -1157,7 +1184,10 @@ static void wsc_free(void *userdata)
 
 	if (wsc->pending_cancel)
 		dbus_pending_reply(&wsc->pending_cancel,
-				dbus_error_aborted(wsc->pending_cancel));
+				dbus_error_not_available(wsc->pending_cancel));
+
+	if (wsc->enrollee)
+		wsc_enrollee_destroy(wsc->enrollee);
 
 	l_free(wsc);
 }
diff --git a/src/wsc.h b/src/wsc.h
index ada08c89..14084aea 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -31,5 +31,14 @@ struct wsc_credentials_info {
 	bool has_passphrase;
 };
 
+struct wsc_enrollee;
+
 typedef void (*wsc_done_cb_t)(int err, struct wsc_credentials_info *creds,
 				unsigned int n_creds, void *user_data);
+
+struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
+					struct scan_bss *target,
+					const char *pin,
+					wsc_done_cb_t done_cb, void *user_data);
+void wsc_enrollee_cancel(struct wsc_enrollee *wsce, bool defer_cb);
+void wsc_enrollee_destroy(struct wsc_enrollee *wsce);
-- 
2.20.1

[PATCH 2/3] wsc: Refactor WSC D-Bus interface logic

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 19541 bytes --]

Split the WSC D-Bus interface class (struct wsc) into a base class
common to station mode and P2P mode (struct wsc_dbus) and station-
specific logic like scanning, saving the credentials as a known network
and triggering the station-mode connection (struct wsc_station_dbus).

Make the base class and its utilities public in wsc.h for P2P use.
---
 src/wsc.c | 363 +++++++++++++++++++++++++++++++++---------------------
 src/wsc.h |  13 ++
 2 files changed, 236 insertions(+), 140 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index e381b84f..4f079977 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -61,20 +61,6 @@ struct wsc_enrollee {
 	bool disconnecting : 1;
 };
 
-struct wsc {
-	struct netdev *netdev;
-	struct station *station;
-	struct wsc_enrollee *enrollee;
-	struct l_dbus_message *pending;
-	struct l_dbus_message *pending_cancel;
-	uint8_t *wsc_ies;
-	size_t wsc_ies_size;
-	struct l_timeout *walk_timer;
-	uint32_t scan_id;
-	struct scan_bss *target;
-	uint32_t station_state_watch;
-};
-
 static struct l_dbus_message *wsc_error_session_overlap(
 						struct l_dbus_message *msg)
 {
@@ -469,7 +455,20 @@ void wsc_enrollee_destroy(struct wsc_enrollee *wsce)
 	wsc_enrollee_free(wsce);
 }
 
-static void wsc_try_credentials(struct wsc *wsc,
+struct wsc_station_dbus {
+	struct wsc_dbus super;
+	struct wsc_enrollee *enrollee;
+	struct scan_bss *target;
+	struct netdev *netdev;
+	struct station *station;
+	uint8_t *wsc_ies;
+	size_t wsc_ies_size;
+	struct l_timeout *walk_timer;
+	uint32_t scan_id;
+	uint32_t station_state_watch;
+};
+
+static void wsc_try_credentials(struct wsc_station_dbus *wsc,
 				struct wsc_credentials_info *creds,
 				unsigned int n_creds)
 {
@@ -510,15 +509,15 @@ static void wsc_try_credentials(struct wsc *wsc,
 		}
 
 		station_connect_network(wsc->station, network, bss,
-								wsc->pending);
-		l_dbus_message_unref(wsc->pending);
-		wsc->pending = NULL;
+						wsc->super.pending_connect);
+		l_dbus_message_unref(wsc->super.pending_connect);
+		wsc->super.pending_connect = NULL;
 
 		return;
 	}
 
-	dbus_pending_reply(&wsc->pending,
-					wsc_error_not_reachable(wsc->pending));
+	dbus_pending_reply(&wsc->super.pending_connect,
+			wsc_error_not_reachable(wsc->super.pending_connect));
 	station_set_autoconnect(wsc->station, true);
 }
 
@@ -559,11 +558,10 @@ static void wsc_store_credentials(struct wsc_credentials_info *creds,
 	}
 }
 
-/* Done callback for when WSC is triggered by DBus methods */
 static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
 				unsigned int n_creds, void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_station_dbus *wsc = user_data;
 
 	wsc->enrollee = NULL;
 	wsc->target = NULL;
@@ -578,27 +576,31 @@ static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
 		break;
 	case -ECANCELED:
 		/* Send reply if we haven't already sent one e.g. in Cancel() */
-		if (wsc->pending)
-			dbus_pending_reply(&wsc->pending,
-					dbus_error_aborted(wsc->pending));
+		if (wsc->super.pending_connect)
+			dbus_pending_reply(&wsc->super.pending_connect,
+					dbus_error_aborted(
+						wsc->super.pending_connect));
 
-		if (wsc->pending_cancel)
-			dbus_pending_reply(&wsc->pending_cancel,
+		if (wsc->super.pending_cancel)
+			dbus_pending_reply(&wsc->super.pending_cancel,
 					l_dbus_message_new_method_return(
-							wsc->pending_cancel));
+						wsc->super.pending_cancel));
 
 		return;
 	case -ENOKEY:
-		dbus_pending_reply(&wsc->pending,
-					wsc_error_no_credentials(wsc->pending));
+		dbus_pending_reply(&wsc->super.pending_connect,
+					wsc_error_no_credentials(
+						wsc->super.pending_connect));
 		return;
 	case -EBUSY:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_busy(wsc->pending));
+		dbus_pending_reply(&wsc->super.pending_connect,
+					dbus_error_busy(
+						wsc->super.pending_connect));
 		return;
 	default:
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_failed(wsc->pending));
+		dbus_pending_reply(&wsc->super.pending_connect,
+					dbus_error_failed(
+						wsc->super.pending_connect));
 		return;
 	}
 
@@ -606,12 +608,14 @@ static void wsc_dbus_done_cb(int err, struct wsc_credentials_info *creds,
 	wsc_try_credentials(wsc, creds, n_creds);
 }
 
-static void wsc_connect(struct wsc *wsc)
+static void wsc_connect(struct wsc_station_dbus *wsc)
 {
 	const char *pin = NULL;
 
-	if (!strcmp(l_dbus_message_get_member(wsc->pending), "StartPin"))
-		l_dbus_message_get_arguments(wsc->pending, "s", &pin);
+	if (!strcmp(l_dbus_message_get_member(wsc->super.pending_connect),
+			"StartPin"))
+		l_dbus_message_get_arguments(wsc->super.pending_connect, "s",
+						&pin);
 
 	wsc->enrollee = wsc_enrollee_new(wsc->netdev, wsc->target, pin,
 						wsc_dbus_done_cb, wsc);
@@ -623,7 +627,7 @@ static void wsc_connect(struct wsc *wsc)
 
 static void station_state_watch(enum station_state state, void *userdata)
 {
-	struct wsc *wsc = userdata;
+	struct wsc_station_dbus *wsc = userdata;
 
 	if (state != STATION_STATE_DISCONNECTED)
 		return;
@@ -636,7 +640,8 @@ static void station_state_watch(enum station_state state, void *userdata)
 	wsc_connect(wsc);
 }
 
-static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
+static void wsc_check_can_connect(struct wsc_station_dbus *wsc,
+					struct scan_bss *target)
 {
 	l_debug("%p", wsc);
 
@@ -671,10 +676,11 @@ static void wsc_check_can_connect(struct wsc *wsc, struct scan_bss *target)
 	}
 error:
 	wsc->target = NULL;
-	dbus_pending_reply(&wsc->pending, dbus_error_failed(wsc->pending));
+	dbus_pending_reply(&wsc->super.pending_connect,
+				dbus_error_failed(wsc->super.pending_connect));
 }
 
-static void wsc_cancel_scan(struct wsc *wsc)
+static void wsc_cancel_scan(struct wsc_station_dbus *wsc)
 {
 	l_free(wsc->wsc_ies);
 	wsc->wsc_ies = 0;
@@ -692,30 +698,32 @@ static void wsc_cancel_scan(struct wsc *wsc)
 
 static void walk_timeout(struct l_timeout *timeout, void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_station_dbus *wsc = user_data;
 
 	wsc_cancel_scan(wsc);
 
-	if (wsc->pending)
-		dbus_pending_reply(&wsc->pending,
-				wsc_error_walk_time_expired(wsc->pending));
+	if (wsc->super.pending_connect)
+		dbus_pending_reply(&wsc->super.pending_connect,
+				wsc_error_walk_time_expired(
+						wsc->super.pending_connect));
 }
 
 static void pin_timeout(struct l_timeout *timeout, void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_station_dbus *wsc = user_data;
 
 	wsc_cancel_scan(wsc);
 
-	if (wsc->pending)
-		dbus_pending_reply(&wsc->pending,
-					wsc_error_time_expired(wsc->pending));
+	if (wsc->super.pending_connect)
+		dbus_pending_reply(&wsc->super.pending_connect,
+				wsc_error_time_expired(
+						wsc->super.pending_connect));
 }
 
 static bool push_button_scan_results(int err, struct l_queue *bss_list,
 					void *userdata)
 {
-	struct wsc *wsc = userdata;
+	struct wsc_station_dbus *wsc = userdata;
 	struct scan_bss *bss_2g;
 	struct scan_bss *bss_5g;
 	struct scan_bss *target;
@@ -726,8 +734,8 @@ static bool push_button_scan_results(int err, struct l_queue *bss_list,
 
 	if (err) {
 		wsc_cancel_scan(wsc);
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_failed(wsc->pending));
+		dbus_pending_reply(&wsc->super.pending_connect,
+				dbus_error_failed(wsc->super.pending_connect));
 
 		return false;
 	}
@@ -828,8 +836,8 @@ static bool push_button_scan_results(int err, struct l_queue *bss_list,
 
 session_overlap:
 	wsc_cancel_scan(wsc);
-	dbus_pending_reply(&wsc->pending,
-				wsc_error_session_overlap(wsc->pending));
+	dbus_pending_reply(&wsc->super.pending_connect,
+			wsc_error_session_overlap(wsc->super.pending_connect));
 
 	return false;
 }
@@ -873,15 +881,15 @@ static bool pin_scan_results(int err, struct l_queue *bss_list, void *userdata)
 {
 	static const uint8_t wildcard_address[] =
 					{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-	struct wsc *wsc = userdata;
+	struct wsc_station_dbus *wsc = userdata;
 	struct scan_bss *target = NULL;
 	const struct l_queue_entry *bss_entry;
 	struct wsc_probe_response probe_response;
 
 	if (err) {
 		wsc_cancel_scan(wsc);
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_failed(wsc->pending));
+		dbus_pending_reply(&wsc->super.pending_connect,
+				dbus_error_failed(wsc->super.pending_connect));
 
 		return false;
 	}
@@ -973,7 +981,7 @@ static bool pin_scan_results(int err, struct l_queue *bss_list, void *userdata)
 	return true;
 }
 
-static bool wsc_initiate_scan(struct wsc *wsc,
+static bool wsc_initiate_scan(struct wsc_station_dbus *wsc,
 					enum wsc_device_password_id dpid,
 					scan_notify_func_t callback)
 {
@@ -1037,28 +1045,117 @@ static bool wsc_initiate_scan(struct wsc *wsc,
 	return true;
 }
 
+static const char *wsc_station_dbus_get_path(struct wsc_dbus *super)
+{
+	struct wsc_station_dbus *wsc =
+		l_container_of(super, struct wsc_station_dbus, super);
+
+	return netdev_get_path(wsc->netdev);
+}
+
+static void wsc_station_dbus_connect(struct wsc_dbus *super,
+						const char *pin)
+{
+	struct wsc_station_dbus *wsc =
+		l_container_of(super, struct wsc_station_dbus, super);
+	scan_notify_func_t scan_callback;
+	enum wsc_device_password_id dpid;
+
+	wsc->station = station_find(netdev_get_ifindex(wsc->netdev));
+	if (!wsc->station) {
+		dbus_pending_reply(&wsc->super.pending_connect,
+					dbus_error_not_available(
+						wsc->super.pending_connect));
+		return;
+	}
+
+	if (pin) {
+		if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
+			dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
+		else
+			dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
+
+		scan_callback = pin_scan_results;
+	} else {
+		dpid = WSC_DEVICE_PASSWORD_ID_PUSH_BUTTON;
+		scan_callback = push_button_scan_results;
+	}
+
+	if (!wsc_initiate_scan(wsc, dpid, scan_callback)) {
+		dbus_pending_reply(&wsc->super.pending_connect,
+					dbus_error_failed(
+						wsc->super.pending_connect));
+		return;
+	}
+
+	if (pin) {
+		wsc->walk_timer = l_timeout_create(60, pin_timeout, wsc, NULL);
+	} else {
+		wsc->walk_timer = l_timeout_create(WALK_TIME, walk_timeout, wsc,
+							NULL);
+	}
+}
+
+static void wsc_station_dbus_cancel(struct wsc_dbus *super)
+{
+	struct wsc_station_dbus *wsc =
+		l_container_of(super, struct wsc_station_dbus, super);
+
+	wsc_cancel_scan(wsc);
+
+	if (wsc->station_state_watch) {
+		station_remove_state_watch(wsc->station,
+						wsc->station_state_watch);
+		wsc->station_state_watch = 0;
+		wsc->target = NULL;
+	}
+
+	dbus_pending_reply(&wsc->super.pending_connect,
+				dbus_error_aborted(wsc->super.pending_connect));
+
+	if (wsc->enrollee)
+		wsc_enrollee_cancel(wsc->enrollee, true);
+	else
+		dbus_pending_reply(&wsc->super.pending_cancel,
+					l_dbus_message_new_method_return(
+						wsc->super.pending_cancel));
+}
+
+static void wsc_station_dbus_remove(struct wsc_dbus *super)
+{
+	struct wsc_station_dbus *wsc =
+		l_container_of(super, struct wsc_station_dbus, super);
+
+	wsc_cancel_scan(wsc);
+
+	if (wsc->station_state_watch) {
+		station_remove_state_watch(wsc->station,
+						wsc->station_state_watch);
+		wsc->station_state_watch = 0;
+	}
+
+	if (wsc->enrollee)
+		wsc_enrollee_destroy(wsc->enrollee);
+
+	l_free(wsc);
+}
+
 static struct l_dbus_message *wsc_push_button(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_dbus *wsc = user_data;
 
 	l_debug("");
 
-	if (wsc->pending)
-		return dbus_error_busy(message);
-
-	wsc->station = station_find(netdev_get_ifindex(wsc->netdev));
-	if (!wsc->station)
-		return dbus_error_not_available(message);
-
-	if (!wsc_initiate_scan(wsc, WSC_DEVICE_PASSWORD_ID_PUSH_BUTTON,
-				push_button_scan_results))
-		return dbus_error_failed(message);
+	if (!l_dbus_message_get_arguments(message, ""))
+		return dbus_error_invalid_args(message);
 
-	wsc->walk_timer = l_timeout_create(WALK_TIME, walk_timeout, wsc, NULL);
-	wsc->pending = l_dbus_message_ref(message);
+	if (wsc->pending_connect)
+		return dbus_error_busy(message);
 
+	wsc->pending_connect = l_dbus_message_ref(message);
+	wsc->connect(wsc, NULL);
 	return NULL;
 }
 
@@ -1066,13 +1163,13 @@ static struct l_dbus_message *wsc_generate_pin(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_dbus *wsc = user_data;
 	struct l_dbus_message *reply;
 	char pin[9];
 
 	l_debug("");
 
-	if (wsc->pending)
+	if (wsc->pending_connect)
 		return dbus_error_busy(message);
 
 	if (!wsc_pin_generate(pin))
@@ -1088,36 +1185,22 @@ static struct l_dbus_message *wsc_start_pin(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_dbus *wsc = user_data;
 	const char *pin;
-	enum wsc_device_password_id dpid;
 
 	l_debug("");
 
-	if (wsc->pending)
+	if (wsc->pending_connect)
 		return dbus_error_busy(message);
 
-	wsc->station = station_find(netdev_get_ifindex(wsc->netdev));
-	if (!wsc->station)
-		return dbus_error_not_available(message);
-
 	if (!l_dbus_message_get_arguments(message, "s", &pin))
 		return dbus_error_invalid_args(message);
 
 	if (!wsc_pin_is_valid(pin))
 		return dbus_error_invalid_format(message);
 
-	if (strlen(pin) == 4 || wsc_pin_is_checksum_valid(pin))
-		dpid = WSC_DEVICE_PASSWORD_ID_DEFAULT;
-	else
-		dpid = WSC_DEVICE_PASSWORD_ID_USER_SPECIFIED;
-
-	if (!wsc_initiate_scan(wsc, dpid, pin_scan_results))
-		return dbus_error_failed(message);
-
-	wsc->walk_timer = l_timeout_create(60, pin_timeout, wsc, NULL);
-	wsc->pending = l_dbus_message_ref(message);
-
+	wsc->pending_connect = l_dbus_message_ref(message);
+	wsc->connect(wsc, pin);
 	return NULL;
 }
 
@@ -1125,33 +1208,22 @@ static struct l_dbus_message *wsc_cancel(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
 {
-	struct wsc *wsc = user_data;
+	struct wsc_dbus *wsc = user_data;
 
 	l_debug("");
 
-	if (!wsc->pending)
+	if (!l_dbus_message_get_arguments(message, ""))
+		return dbus_error_invalid_args(message);
+
+	if (!wsc->pending_connect)
 		return dbus_error_not_available(message);
 
 	if (wsc->pending_cancel)
 		return dbus_error_busy(message);
 
-	wsc_cancel_scan(wsc);
-
-	if (wsc->station_state_watch) {
-		station_remove_state_watch(wsc->station,
-						wsc->station_state_watch);
-		wsc->station_state_watch = 0;
-		wsc->target = NULL;
-	}
-
-	dbus_pending_reply(&wsc->pending, dbus_error_aborted(wsc->pending));
-
-	if (wsc->enrollee) {
-		wsc_enrollee_cancel(wsc->enrollee, true);
-		wsc->pending_cancel = l_dbus_message_ref(message);
-		return NULL;
-	} else
-		return l_dbus_message_new_method_return(message);
+	wsc->pending_cancel = l_dbus_message_ref(message);
+	wsc->cancel(wsc);
+	return NULL;
 }
 
 static void setup_wsc_interface(struct l_dbus_interface *interface)
@@ -1166,36 +1238,47 @@ static void setup_wsc_interface(struct l_dbus_interface *interface)
 				wsc_cancel, "", "");
 }
 
-static void wsc_free(void *userdata)
+bool wsc_dbus_add_interface(struct wsc_dbus *wsc)
 {
-	struct wsc *wsc = userdata;
-
-	wsc_cancel_scan(wsc);
+	struct l_dbus *dbus = dbus_get_bus();
 
-	if (wsc->station_state_watch) {
-		station_remove_state_watch(wsc->station,
-						wsc->station_state_watch);
-		wsc->station_state_watch = 0;
+	if (!l_dbus_object_add_interface(dbus, wsc->get_path(wsc),
+						IWD_WSC_INTERFACE, wsc)) {
+		l_info("Unable to register %s interface", IWD_WSC_INTERFACE);
+		return false;
 	}
 
-	if (wsc->pending)
-		dbus_pending_reply(&wsc->pending,
-					dbus_error_not_available(wsc->pending));
+	return true;
+}
+
+void wsc_dbus_remove_interface(struct wsc_dbus *wsc)
+{
+	struct l_dbus *dbus = dbus_get_bus();
+
+	l_dbus_object_remove_interface(dbus, wsc->get_path(wsc),
+					IWD_WSC_INTERFACE);
+}
+
+static void wsc_dbus_free(void *user_data)
+{
+	struct wsc_dbus *wsc = user_data;
+
+	if (wsc->pending_connect)
+		dbus_pending_reply(&wsc->pending_connect,
+					dbus_error_not_available(
+						wsc->pending_connect));
 
 	if (wsc->pending_cancel)
 		dbus_pending_reply(&wsc->pending_cancel,
-				dbus_error_not_available(wsc->pending_cancel));
-
-	if (wsc->enrollee)
-		wsc_enrollee_destroy(wsc->enrollee);
+					dbus_error_not_available(
+						wsc->pending_cancel));
 
-	l_free(wsc);
+	wsc->remove(wsc);
 }
 
-static void wsc_add_interface(struct netdev *netdev)
+static void wsc_add_station(struct netdev *netdev)
 {
-	struct l_dbus *dbus = dbus_get_bus();
-	struct wsc *wsc;
+	struct wsc_station_dbus *wsc;
 
 	if (!wiphy_get_max_scan_ie_len(netdev_get_wiphy(netdev))) {
 		l_debug("Simple Configuration isn't supported by ifindex %u",
@@ -1204,18 +1287,18 @@ static void wsc_add_interface(struct netdev *netdev)
 		return;
 	}
 
-	wsc = l_new(struct wsc, 1);
+	wsc = l_new(struct wsc_station_dbus, 1);
 	wsc->netdev = netdev;
+	wsc->super.get_path = wsc_station_dbus_get_path;
+	wsc->super.connect = wsc_station_dbus_connect;
+	wsc->super.cancel = wsc_station_dbus_cancel;
+	wsc->super.remove = wsc_station_dbus_remove;
 
-	if (!l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
-						IWD_WSC_INTERFACE,
-						wsc)) {
-		wsc_free(wsc);
-		l_info("Unable to register %s interface", IWD_WSC_INTERFACE);
-	}
+	if (!wsc_dbus_add_interface(&wsc->super))
+		wsc_station_dbus_remove(&wsc->super);
 }
 
-static void wsc_remove_interface(struct netdev *netdev)
+static void wsc_remove_station(struct netdev *netdev)
 {
 	struct l_dbus *dbus = dbus_get_bus();
 
@@ -1231,11 +1314,11 @@ static void wsc_netdev_watch(struct netdev *netdev,
 	case NETDEV_WATCH_EVENT_NEW:
 		if (netdev_get_iftype(netdev) == NETDEV_IFTYPE_STATION &&
 				netdev_get_is_up(netdev))
-			wsc_add_interface(netdev);
+			wsc_add_station(netdev);
 		break;
 	case NETDEV_WATCH_EVENT_DOWN:
 	case NETDEV_WATCH_EVENT_DEL:
-		wsc_remove_interface(netdev);
+		wsc_remove_station(netdev);
 		break;
 	default:
 		break;
@@ -1248,7 +1331,7 @@ static int wsc_init(void)
 	netdev_watch = netdev_watch_add(wsc_netdev_watch, NULL, NULL);
 	l_dbus_register_interface(dbus_get_bus(), IWD_WSC_INTERFACE,
 					setup_wsc_interface,
-					wsc_free, false);
+					wsc_dbus_free, false);
 	return 0;
 }
 
diff --git a/src/wsc.h b/src/wsc.h
index 14084aea..fdf677b6 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -42,3 +42,16 @@ struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 					wsc_done_cb_t done_cb, void *user_data);
 void wsc_enrollee_cancel(struct wsc_enrollee *wsce, bool defer_cb);
 void wsc_enrollee_destroy(struct wsc_enrollee *wsce);
+
+struct wsc_dbus {
+	struct l_dbus_message *pending_connect;
+	struct l_dbus_message *pending_cancel;
+
+	const char *(*get_path)(struct wsc_dbus *wsc);
+	void (*connect)(struct wsc_dbus *wsc, const char *pin);
+	void (*cancel)(struct wsc_dbus *wsc);
+	void (*remove)(struct wsc_dbus *wsc);
+};
+
+bool wsc_dbus_add_interface(struct wsc_dbus *wsc);
+void wsc_dbus_remove_interface(struct wsc_dbus *wsc);
-- 
2.20.1

[PATCH 3/3] wsc: Accept extra IEs in wsc_enrollee_new

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3185 bytes --]

---
 src/wsc.c | 23 ++++++++++++++---------
 src/wsc.h |  1 +
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index 4f079977..8f367c7c 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -324,7 +324,8 @@ static inline enum wsc_rf_band freq_to_rf_band(uint32_t freq)
 }
 
 static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
-					const char *pin)
+					const char *pin, struct iovec *ies,
+					unsigned int ies_num)
 {
 	struct handshake_state *hs;
 	struct l_settings *settings = l_settings_new();
@@ -332,7 +333,7 @@ static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
 	struct wsc_association_request request;
 	uint8_t *pdu;
 	size_t pdu_len;
-	struct iovec ie_iov;
+	struct iovec ie_iov[1 + ies_num];
 
 	hs = netdev_handshake_state_new(wsce->netdev);
 
@@ -376,19 +377,22 @@ static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
 		goto error;
 	}
 
-	ie_iov.iov_base = ie_tlv_encapsulate_wsc_payload(pdu, pdu_len,
-							&ie_iov.iov_len);
+	ie_iov[0].iov_base = ie_tlv_encapsulate_wsc_payload(pdu, pdu_len,
+							&ie_iov[0].iov_len);
 	l_free(pdu);
 
-	if (!ie_iov.iov_base) {
+	if (!ie_iov[0].iov_base) {
 		r = -ENOMEM;
 		goto error;
 	}
 
-	r = netdev_connect(wsce->netdev, bss, hs, &ie_iov, 1,
+	if (ies_num)
+		memcpy(ie_iov + 1, ies, sizeof(struct iovec) * ies_num);
+
+	r = netdev_connect(wsce->netdev, bss, hs, ie_iov, 1 + ies_num,
 				wsc_enrollee_netdev_event,
 				wsc_enrollee_connect_cb, wsce);
-	l_free(ie_iov.iov_base);
+	l_free(ie_iov[0].iov_base);
 
 	if (r == 0)
 		return 0;
@@ -401,6 +405,7 @@ error:
 struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 					struct scan_bss *target,
 					const char *pin,
+					struct iovec *ies, unsigned int ies_num,
 					wsc_done_cb_t done_cb, void *user_data)
 {
 	struct wsc_enrollee *wsce;
@@ -410,7 +415,7 @@ struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 	wsce->done_cb = done_cb;
 	wsce->done_data = user_data;
 
-	if (wsc_enrollee_connect(wsce, target, pin) == 0)
+	if (wsc_enrollee_connect(wsce, target, pin, ies, ies_num) == 0)
 		return wsce;
 
 	wsc_enrollee_free(wsce);
@@ -617,7 +622,7 @@ static void wsc_connect(struct wsc_station_dbus *wsc)
 		l_dbus_message_get_arguments(wsc->super.pending_connect, "s",
 						&pin);
 
-	wsc->enrollee = wsc_enrollee_new(wsc->netdev, wsc->target, pin,
+	wsc->enrollee = wsc_enrollee_new(wsc->netdev, wsc->target, pin, NULL, 0,
 						wsc_dbus_done_cb, wsc);
 	if (wsc->enrollee)
 		return;
diff --git a/src/wsc.h b/src/wsc.h
index fdf677b6..6596bb00 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -39,6 +39,7 @@ typedef void (*wsc_done_cb_t)(int err, struct wsc_credentials_info *creds,
 struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 					struct scan_bss *target,
 					const char *pin,
+					struct iovec *ies, unsigned int ies_num,
 					wsc_done_cb_t done_cb, void *user_data);
 void wsc_enrollee_cancel(struct wsc_enrollee *wsce, bool defer_cb);
 void wsc_enrollee_destroy(struct wsc_enrollee *wsce);
-- 
2.20.1

Re: [PATCH 1/3] wsc: Split out enrollee state machine to own object

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

Hi Andrew,

On 1/15/20 10:29 PM, Andrew Zaborowski wrote:
> Create struct wsc_enrollee which is allocated with wsc_enrollee_new,
> taking a done callback as a parameter.  The callback is always
> called so there's no need for a separate destroy callback.  The object
> only lives until the done callback happens so wsc_enrollee_cancel/destroy
> can only be used before this.
> 
> Looks like the rest of the file is simplified thanks to this.
> ---
>   src/wsc.c | 504 +++++++++++++++++++++++++++++-------------------------
>   src/wsc.h |   9 +
>   2 files changed, 276 insertions(+), 237 deletions(-)
>
<snip>


> @@ -1157,7 +1184,10 @@ static void wsc_free(void *userdata)
>   
>   	if (wsc->pending_cancel)
>   		dbus_pending_reply(&wsc->pending_cancel,
> -				dbus_error_aborted(wsc->pending_cancel));
> +				dbus_error_not_available(wsc->pending_cancel));

I left this as error_aborted for now.  I'd rather not do so without a 
good reason given that the API is frozen.  At the very least, lets do 
this in a separate patch so it is visible / documented.    If you think 
it should be not_available, please send a separate patch.

> +
> +	if (wsc->enrollee)
> +		wsc_enrollee_destroy(wsc->enrollee);
>   
>   	l_free(wsc);
>   }

Otherwise, everything in this series has now been applied, thanks!  I 
did push out a few tweaks on top, please review.

Regards,
-Denis

[PATCH 3/3] wsc: Accept extra IEs in wsc_enrollee_new

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3117 bytes --]

---
 src/wsc.c | 23 ++++++++++++++---------
 src/wsc.h |  1 +
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/wsc.c b/src/wsc.c
index 74fe8aa8..56018825 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -320,7 +320,8 @@ static inline enum wsc_rf_band freq_to_rf_band(uint32_t freq)
 }
 
 static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
-					const char *pin)
+					const char *pin, struct iovec *ies,
+					unsigned int ies_num)
 {
 	struct handshake_state *hs;
 	struct l_settings *settings = l_settings_new();
@@ -328,7 +329,7 @@ static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
 	struct wsc_association_request request;
 	uint8_t *pdu;
 	size_t pdu_len;
-	struct iovec ie_iov;
+	struct iovec ie_iov[1 + ies_num];
 
 	hs = netdev_handshake_state_new(wsce->netdev);
 
@@ -372,19 +373,22 @@ static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
 		goto error;
 	}
 
-	ie_iov.iov_base = ie_tlv_encapsulate_wsc_payload(pdu, pdu_len,
-							&ie_iov.iov_len);
+	ie_iov[0].iov_base = ie_tlv_encapsulate_wsc_payload(pdu, pdu_len,
+							&ie_iov[0].iov_len);
 	l_free(pdu);
 
-	if (!ie_iov.iov_base) {
+	if (!ie_iov[0].iov_base) {
 		r = -ENOMEM;
 		goto error;
 	}
 
-	r = netdev_connect(wsce->netdev, bss, hs, &ie_iov, 1,
+	if (ies_num)
+		memcpy(ie_iov + 1, ies, sizeof(struct iovec) * ies_num);
+
+	r = netdev_connect(wsce->netdev, bss, hs, ie_iov, 1 + ies_num,
 				wsc_enrollee_netdev_event,
 				wsc_enrollee_connect_cb, wsce);
-	l_free(ie_iov.iov_base);
+	l_free(ie_iov[0].iov_base);
 
 	if (r == 0)
 		return 0;
@@ -397,6 +401,7 @@ error:
 struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 					struct scan_bss *target,
 					const char *pin,
+					struct iovec *ies, unsigned int ies_num,
 					wsc_done_cb_t done_cb, void *user_data)
 {
 	struct wsc_enrollee *wsce;
@@ -406,7 +411,7 @@ struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 	wsce->done_cb = done_cb;
 	wsce->done_data = user_data;
 
-	if (wsc_enrollee_connect(wsce, target, pin) == 0)
+	if (wsc_enrollee_connect(wsce, target, pin, ies, ies_num) == 0)
 		return wsce;
 
 	wsc_enrollee_free(wsce);
@@ -577,7 +582,7 @@ static void wsc_connect(struct wsc_station_dbus *wsc)
 		l_dbus_message_get_arguments(wsc->super.pending_connect, "s",
 						&pin);
 
-	wsc->enrollee = wsc_enrollee_new(wsc->netdev, wsc->target, pin,
+	wsc->enrollee = wsc_enrollee_new(wsc->netdev, wsc->target, pin, NULL, 0,
 						wsc_dbus_done_cb, wsc);
 	if (wsc->enrollee)
 		return;
diff --git a/src/wsc.h b/src/wsc.h
index 3cb7ab18..570aa7cb 100644
--- a/src/wsc.h
+++ b/src/wsc.h
@@ -39,6 +39,7 @@ typedef void (*wsc_done_cb_t)(int err, struct wsc_credentials_info *creds,
 struct wsc_enrollee *wsc_enrollee_new(struct netdev *netdev,
 					struct scan_bss *target,
 					const char *pin,
+					struct iovec *ies, unsigned int ies_num,
 					wsc_done_cb_t done_cb, void *user_data);
 void wsc_enrollee_cancel(struct wsc_enrollee *wsce);
 
-- 
2.20.1