Remove overly complex hostapd setup sequence with n+1 callbacks

This code was originally added as a mechanism to handle long waits
during channel selection and/or radar detection. It is not currently
really used and makes the setup sequence nearly impossible to
understand. Let's get rid of the unwanted complexity. This needs to be
redesigned if it is ever needed again.

hostapd/hostapd.c

@@ -1385,145 +1385,6 @@ static void hostapd_tx_queue_params(struct hostapd_iface *iface)
 }
 
 
-/**
- * setup_interface2 - Setup (initialize) an interface (part 2)
- * @iface: Pointer to interface data.
- * Returns: 0 on success; -1 on failure.
- *
- * Flushes old stations, sets the channel, DFS parameters, encryption,
- * beacons, and WDS links based on the configuration.
- */
-static int setup_interface2(struct hostapd_iface *iface)
-{
-	struct hostapd_data *hapd = iface->bss[0];
-	int freq;
-	size_t j;
-	int ret = 0;
-	u8 *prev_addr;
-
-	hostapd_flush_old_stations(hapd);
-	hostapd_set_privacy(hapd, 0);
-
-	if (hapd->iconf->channel) {
-		freq = hostapd_hw_get_freq(hapd, hapd->iconf->channel);
-		printf("Mode: %s  Channel: %d  Frequency: %d MHz\n",
-		       hostapd_hw_mode_txt(hapd->iconf->hw_mode),
-		       hapd->iconf->channel, freq);
-
-		if (hostapd_set_freq(hapd, hapd->iconf->hw_mode, freq)) {
-			printf("Could not set channel for kernel driver\n");
-			return -1;
-		}
-	}
-
-	hostapd_broadcast_wep_clear(hapd);
-	if (hostapd_setup_encryption(hapd->conf->iface, hapd))
-		return -1;
-
-	hostapd_set_beacon_int(hapd, hapd->iconf->beacon_int);
-	ieee802_11_set_beacon(hapd);
-
-	if (hapd->iconf->rts_threshold > -1 &&
-	    hostapd_set_rts(hapd, hapd->iconf->rts_threshold)) {
-		printf("Could not set RTS threshold for kernel driver\n");
-		return -1;
-	}
-
-	if (hapd->iconf->fragm_threshold > -1 &&
-	    hostapd_set_frag(hapd, hapd->iconf->fragm_threshold)) {
-		printf("Could not set fragmentation threshold for kernel "
-		       "driver\n");
-		return -1;
-	}
-
-	prev_addr = hapd->own_addr;
-
-	for (j = 0; j < iface->num_bss; j++) {
-		hapd = iface->bss[j];
-		if (j)
-			os_memcpy(hapd->own_addr, prev_addr, ETH_ALEN);
-		if (hostapd_setup_bss(hapd, j == 0))
-			return -1;
-		if (hostapd_mac_comp_empty(hapd->conf->bssid) == 0)
-			prev_addr = hapd->own_addr;
-	}
-
-	hostapd_tx_queue_params(iface);
-
-	ap_list_init(iface);
-
-	if (hostapd_driver_commit(hapd) < 0) {
-		wpa_printf(MSG_ERROR, "%s: Failed to commit driver "
-			   "configuration", __func__);
-		return -1;
-	}
-
-	return ret;
-}
-
-
-static void setup_interface_start(void *eloop_data, void *user_ctx);
-static void setup_interface2_handler(void *eloop_data, void *user_ctx);
-
-/**
- * setup_interface_finalize - Finish setup interface & call the callback
- * @iface: Pointer to interface data.
- * @status: Status of the setup interface (0 on success; -1 on failure).
- * Returns: 0 on success; -1 on failure (e.g., was not in progress).
- */
-static int setup_interface_finalize(struct hostapd_iface *iface, int status)
-{
-	hostapd_iface_cb cb;
-
-	if (!iface->setup_cb)
-		return -1;
-	
-	eloop_cancel_timeout(setup_interface_start, iface, NULL);
-	eloop_cancel_timeout(setup_interface2_handler, iface, NULL);
-	hostapd_select_hw_mode_stop(iface);
-
-	cb = iface->setup_cb;
-
-	iface->setup_cb = NULL;
-
-	cb(iface, status);
-
-	return 0;
-}
-
-
-/**
- * setup_interface2_wrapper - Wrapper for setup_interface2()
- * @iface: Pointer to interface data.
- * @status: Status of the hw mode select.
- *
- * Wrapper for setup_interface2() to calls finalize function upon completion.
- */
-static void setup_interface2_wrapper(struct hostapd_iface *iface, int status)
-{
-	int ret = status;
-	if (ret)
-		printf("Could not select hw_mode and channel. (%d)\n", ret);
-	else
-		ret = setup_interface2(iface);
-
-	setup_interface_finalize(iface, ret);
-}
-
-
-/**
- * setup_interface2_handler - Used for immediate call of setup_interface2
- * @eloop_data: Stores the struct hostapd_iface * for the interface.
- * @user_ctx: Unused.
- */
-static void setup_interface2_handler(void *eloop_data, void *user_ctx)
-{
-	struct hostapd_iface *iface = eloop_data;
-
-	setup_interface2_wrapper(iface, 0);
-}
-
-
 static int hostapd_radius_get_eap_user(void *ctx, const u8 *identity,
 				       size_t identity_len, int phase2,
 				       struct eap_user *user)
@@ -1563,23 +1424,17 @@ static int hostapd_radius_get_eap_user(void *ctx, const u8 *identity,
 }
 
 
-/**
- * setup_interface1 - Setup (initialize) an interface (part 1)
- * @iface: Pointer to interface data
- * Returns: 0 on success, -1 on failure
- *
- * Initializes the driver interface, validates the configuration,
- * and sets driver parameters based on the configuration.
- * Schedules setup_interface2() to be called immediately or after
- * hardware mode setup takes place. 
- */
-static int setup_interface1(struct hostapd_iface *iface)
+static int setup_interface(struct hostapd_iface *iface)
 {
 	struct hostapd_data *hapd = iface->bss[0];
 	struct hostapd_bss_config *conf = hapd->conf;
 	size_t i;
 	char country[4];
 	u8 *b = conf->bssid;
+	int freq;
+	size_t j;
+	int ret = 0;
+	u8 *prev_addr;
 
 	/*
 	 * Initialize the driver interface and make sure that all BSSes get
@@ -1639,77 +1494,104 @@ static int setup_interface1(struct hostapd_iface *iface)
 		/* Not all drivers support this yet, so continue without hw
 		 * feature data. */
 	} else {
-		return hostapd_select_hw_mode_start(iface,
-						    setup_interface2_wrapper);
+		int ret = hostapd_select_hw_mode(iface);
+		if (ret < 0) {
+			printf("Could not select hw_mode and channel. (%d)\n",
+			       ret);
+			return -1;
+		}
 	}
 
-	eloop_register_timeout(0, 0, setup_interface2_handler, iface, NULL);
-	return 0;
-}
+	hostapd_flush_old_stations(hapd);
+	hostapd_set_privacy(hapd, 0);
 
+	if (hapd->iconf->channel) {
+		freq = hostapd_hw_get_freq(hapd, hapd->iconf->channel);
+		printf("Mode: %s  Channel: %d  Frequency: %d MHz\n",
+		       hostapd_hw_mode_txt(hapd->iconf->hw_mode),
+		       hapd->iconf->channel, freq);
 
-/**
- * setup_interface_start - Handler to start setup interface
- * @eloop_data: Stores the struct hostapd_iface * for the interface.
- * @user_ctx: Unused.
- *
- * An eloop handler is used so that all errors can be processed by the
- * callback without introducing stack recursion.
- */
-static void setup_interface_start(void *eloop_data, void *user_ctx)
-{
-	struct hostapd_iface *iface = eloop_data;
+		if (hostapd_set_freq(hapd, hapd->iconf->hw_mode, freq)) {
+			printf("Could not set channel for kernel driver\n");
+			return -1;
+		}
+	}
 
-	int ret;
+	hostapd_broadcast_wep_clear(hapd);
+	if (hostapd_setup_encryption(hapd->conf->iface, hapd))
+		return -1;
+
+	hostapd_set_beacon_int(hapd, hapd->iconf->beacon_int);
+	ieee802_11_set_beacon(hapd);
+
+	if (hapd->iconf->rts_threshold > -1 &&
+	    hostapd_set_rts(hapd, hapd->iconf->rts_threshold)) {
+		printf("Could not set RTS threshold for kernel driver\n");
+		return -1;
+	}
+
+	if (hapd->iconf->fragm_threshold > -1 &&
+	    hostapd_set_frag(hapd, hapd->iconf->fragm_threshold)) {
+		printf("Could not set fragmentation threshold for kernel "
+		       "driver\n");
+		return -1;
+	}
 
-	ret = setup_interface1(iface);
-	if (ret)
-		setup_interface_finalize(iface, ret);
+	prev_addr = hapd->own_addr;
+
+	for (j = 0; j < iface->num_bss; j++) {
+		hapd = iface->bss[j];
+		if (j)
+			os_memcpy(hapd->own_addr, prev_addr, ETH_ALEN);
+		if (hostapd_setup_bss(hapd, j == 0))
+			return -1;
+		if (hostapd_mac_comp_empty(hapd->conf->bssid) == 0)
+			prev_addr = hapd->own_addr;
+	}
+
+	hostapd_tx_queue_params(iface);
+
+	ap_list_init(iface);
+
+	if (hostapd_driver_commit(hapd) < 0) {
+		wpa_printf(MSG_ERROR, "%s: Failed to commit driver "
+			   "configuration", __func__);
+		return -1;
+	}
+
+	return ret;
 }
 
 
 /**
- * hostapd_setup_interface_start - Start the setup of an interface
+ * hostapd_setup_interface - Setup of an interface
  * @iface: Pointer to interface data.
- * @cb: The function to callback when done.
- * Returns:  0 if it starts successfully; cb will be called when done.
- *          -1 on failure; cb will not be called.
+ * Returns: 0 on success, -1 on failure
  *
  * Initializes the driver interface, validates the configuration,
  * and sets driver parameters based on the configuration.
- * Flushes old stations, sets the channel, DFS parameters, encryption,
+ * Flushes old stations, sets the channel, encryption,
  * beacons, and WDS links based on the configuration.
  */
-int hostapd_setup_interface_start(struct hostapd_iface *iface,
-				  hostapd_iface_cb cb)
+static int hostapd_setup_interface(struct hostapd_iface *iface)
 {
-	if (iface->setup_cb) {
-		wpa_printf(MSG_DEBUG,
-			   "%s: Interface setup already in progress.\n",
+	int ret;
+
+	ret = setup_interface(iface);
+	if (ret) {
+		wpa_printf(MSG_DEBUG, "%s: Unable to setup interface.",
 			   iface->bss[0]->conf->iface);
+		eloop_terminate();
 		return -1;
+	} else if (!hostapd_drv_none(iface->bss[0])) {
+		wpa_printf(MSG_DEBUG, "%s: Setup of interface done.",
+			   iface->bss[0]->conf->iface);
 	}
 
-	iface->setup_cb = cb;
-
-	eloop_register_timeout(0, 0, setup_interface_start, iface, NULL);
-
 	return 0;
 }
 
 
-/**
- * hostapd_setup_interace_stop - Stops the setup of an interface
- * @iface: Pointer to interface data
- * Returns:  0 if successfully stopped;
- *          -1 on failure (i.e., was not in progress)
- */
-int hostapd_setup_interface_stop(struct hostapd_iface *iface)
-{
-	return setup_interface_finalize(iface, -1);
-}
-
-
 static void show_version(void)
 {
 	fprintf(stderr,
@@ -1897,23 +1779,6 @@ fail:
 }
 
 
-/**
- * setup_interface_done - Callback when an interface is done being setup.
- * @iface: Pointer to interface data.
- * @status: Status of the interface setup (0 on success; -1 on failure).
- */
-static void setup_interface_done(struct hostapd_iface *iface, int status)
-{
-	if (status) {
-		wpa_printf(MSG_DEBUG, "%s: Unable to setup interface.",
-			   iface->bss[0]->conf->iface);
-		eloop_terminate();
-	} else if (!hostapd_drv_none(iface->bss[0]))
-		wpa_printf(MSG_DEBUG, "%s: Setup of interface done.",
-			   iface->bss[0]->conf->iface);
-}
-
-
 int main(int argc, char *argv[])
 {
 	struct hapd_interfaces interfaces;
@@ -2001,8 +1866,7 @@ int main(int argc, char *argv[])
 					logger_stdout_level--;
 		}
 
-		ret = hostapd_setup_interface_start(interfaces.iface[i],
-						    setup_interface_done);
+		ret = hostapd_setup_interface(interfaces.iface[i]);
 		if (ret)
 			goto out;
 
@@ -2047,7 +1911,6 @@ int main(int argc, char *argv[])
 	for (i = 0; i < interfaces.count; i++) {
 		if (!interfaces.iface[i])
 			continue;
-		hostapd_setup_interface_stop(interfaces.iface[i]);
 		hostapd_cleanup_iface_pre(interfaces.iface[i]);
 		for (j = 0; j < interfaces.iface[i]->num_bss; j++) {
 			struct hostapd_data *hapd =

hostapd/hostapd.h

@@ -1,7 +1,7 @@
 /*
  * hostapd / Initialization and configuration
  * Host AP kernel driver
- * Copyright (c) 2002-2007, Jouni Malinen <j@w1.fi>
+ * Copyright (c) 2002-2008, Jouni Malinen <j@w1.fi>
  * Copyright (c) 2007-2008, Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
@@ -172,16 +172,6 @@ struct hostapd_data {
 };
 
 
-/**
- * hostapd_iface_cb - Generic callback type for per-iface asynchronous requests
- * @iface: the interface the event occured on.
- * @status: 0 if the request succeeded; -1 if the request failed.
- */
-typedef void (*hostapd_iface_cb)(struct hostapd_iface *iface, int status);
-
-
-struct hostapd_config_change;
-
 /**
  * struct hostapd_iface - hostapd per-interface data structure
  */
@@ -189,8 +179,6 @@ struct hostapd_iface {
 	char *config_fname;
 	struct hostapd_config *conf;
 
-	hostapd_iface_cb setup_cb;
-
 	size_t num_bss;
 	struct hostapd_data **bss;
 
@@ -206,7 +194,6 @@ struct hostapd_iface {
 	 * current_mode->channels */
 	int num_rates;
 	struct hostapd_rate_data *current_rates;
-	hostapd_iface_cb hw_mode_sel_cb;
 
 	u16 hw_flags;
 

hostapd/hw_features.c

@@ -2,6 +2,7 @@
  * hostapd / Hardware feature query and different modes
  * Copyright 2002-2003, Instant802 Networks, Inc.
  * Copyright 2005-2006, Devicescape Software, Inc.
+ * Copyright (c) 2008, Jouni Malinen <j@w1.fi>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -19,7 +20,6 @@
 #include "hw_features.h"
 #include "driver.h"
 #include "config.h"
-#include "eloop.h"
 
 
 void hostapd_free_hw_features(struct hostapd_hw_modes *hw_features,
@@ -169,111 +169,17 @@ static int hostapd_prepare_rates(struct hostapd_data *hapd,
 }
 
 
-static void select_hw_mode_start(void *eloop_data, void *user_ctx);
-static void select_hw_mode2_handler(void *eloop_data, void *user_ctx);
-
-/**
- * select_hw_mode_finalize - Finish select HW mode & call the callback
- * @iface: Pointer to interface data.
- * @status: Status of the select HW mode (0 on success; -1 on failure).
- * Returns: 0 on success; -1 on failure (e.g., was not in progress).
- */
-static int select_hw_mode_finalize(struct hostapd_iface *iface, int status)
-{
-	hostapd_iface_cb cb;
-
-	if (!iface->hw_mode_sel_cb)
-		return -1;
-
-	eloop_cancel_timeout(select_hw_mode_start, iface, NULL);
-	eloop_cancel_timeout(select_hw_mode2_handler, iface, NULL);
-
-	cb = iface->hw_mode_sel_cb;
-
-	iface->hw_mode_sel_cb = NULL;
-
-	cb(iface, status);
-
-	return 0;
-}
-
-
-/**
- * select_hw_mode2 - Select the hardware mode (part 2)
- * @iface: Pointer to interface data.
- * @status: Status of auto chanel selection.
- *
- * Setup the rates and passive scanning based on the configuration.
- */
-static void select_hw_mode2(struct hostapd_iface *iface, int status)
-{
-	int ret = status;
-	if (ret)
-		goto fail;
-
-	if (iface->current_mode == NULL) {
-		hostapd_logger(iface->bss[0], NULL, HOSTAPD_MODULE_IEEE80211,
-			       HOSTAPD_LEVEL_WARNING,
-			       "Hardware does not support configured channel");
-		ret = -1;
-		goto fail;
-	}
-
-	if (hostapd_prepare_rates(iface->bss[0], iface->current_mode)) {
-		wpa_printf(MSG_ERROR, "Failed to prepare rates table.");
-		hostapd_logger(iface->bss[0], NULL, HOSTAPD_MODULE_IEEE80211,
-					   HOSTAPD_LEVEL_WARNING,
-					   "Failed to prepare rates table.");
-		ret = -1;
-		goto fail;
-	}
-
-	ret = hostapd_passive_scan(iface->bss[0], 0,
-				   iface->conf->passive_scan_mode,
-				   iface->conf->passive_scan_interval,
-				   iface->conf->passive_scan_listen,
-				   NULL, NULL);
-	if (ret) {
-		if (ret == -1) {
-			wpa_printf(MSG_DEBUG, "Passive scanning not "
-				   "supported");
-		} else {
-			wpa_printf(MSG_ERROR, "Could not set passive "
-				   "scanning: %s", strerror(ret));
-		}
-		ret = 0;
-	}
-
-fail:
-	select_hw_mode_finalize(iface, ret);
-}
-
-
-/**
- * select_hw_mode2_handler - Calls select_hw_mode2 when auto chan isn't used
- * @eloop_data: Stores the struct hostapd_iface * for the interface.
- * @user_ctx: Unused.
- */
-static void select_hw_mode2_handler(void *eloop_data, void *user_ctx)
-{
-	struct hostapd_iface *iface = eloop_data;
-
-	select_hw_mode2(iface, 0);
-}
-
-
 /**
- * select_hw_mode1 - Select the hardware mode (part 1)
+ * hostapd_select_hw_mode - Select the hardware mode
  * @iface: Pointer to interface data.
- * Returns: 0 on success; -1 on failure.
+ * Returns: 0 on success, -1 on failure
  *
- * Setup the hardware mode and channel based on the configuration.
- * Schedules select_hw_mode2() to be called immediately or after automatic
- * channel selection takes place.
+ * Sets up the hardware mode, channel, rates, and passive scanning
+ * based on the configuration.
  */
-static int select_hw_mode1(struct hostapd_iface *iface)
+int hostapd_select_hw_mode(struct hostapd_iface *iface)
 {
-	int i, j, ok;
+	int i, j, ok, ret;
 
 	if (iface->num_hw_features < 1)
 		return -1;
@@ -319,72 +225,38 @@ static int select_hw_mode1(struct hostapd_iface *iface)
 		iface->current_mode = NULL;
 	}
 
-	/*
-	 * Calls select_hw_mode2() via a handler, so that the function is
-	 * always executed from eloop.
-	 */
-	eloop_register_timeout(0, 0, select_hw_mode2_handler, iface, NULL);
-	return 0;
-}
-
-
-/**
- * select_hw_mode_start - Handler to start select HW mode
- * @eloop_data: Stores the struct hostapd_iface * for the interface.
- * @user_ctx: Unused.
- *
- * An eloop handler is used so that all errors can be processed by the
- * callback without introducing stack recursion.
- */
-static void select_hw_mode_start(void *eloop_data, void *user_ctx)
-{
-	struct hostapd_iface *iface = (struct hostapd_iface *)eloop_data;
-
-	int ret;
-
-	ret = select_hw_mode1(iface);
-	if (ret)
-		select_hw_mode_finalize(iface, ret);
-}
-
-
-/**
- * hostapd_select_hw_mode_start - Start selection of the hardware mode
- * @iface: Pointer to interface data.
- * @cb: The function to callback when done.
- * Returns:  0 if it starts successfully; cb will be called when done.
- *          -1 on failure; cb will not be called.
- *
- * Sets up the hardware mode, channel, rates, and passive scanning
- * based on the configuration.
- */
-int hostapd_select_hw_mode_start(struct hostapd_iface *iface,
-				 hostapd_iface_cb cb)
-{
-	if (iface->hw_mode_sel_cb) {
-		wpa_printf(MSG_DEBUG,
-			   "%s: Hardware mode select already in progress.",
-			   iface->bss[0]->conf->iface);
+	if (iface->current_mode == NULL) {
+		hostapd_logger(iface->bss[0], NULL, HOSTAPD_MODULE_IEEE80211,
+			       HOSTAPD_LEVEL_WARNING,
+			       "Hardware does not support configured channel");
 		return -1;
 	}
 
-	iface->hw_mode_sel_cb = cb;
-
-	eloop_register_timeout(0, 0, select_hw_mode_start, iface, NULL);
-
-	return 0;
-}
+	if (hostapd_prepare_rates(iface->bss[0], iface->current_mode)) {
+		wpa_printf(MSG_ERROR, "Failed to prepare rates table.");
+		hostapd_logger(iface->bss[0], NULL, HOSTAPD_MODULE_IEEE80211,
+					   HOSTAPD_LEVEL_WARNING,
+					   "Failed to prepare rates table.");
+		return -1;
+	}
 
+	ret = hostapd_passive_scan(iface->bss[0], 0,
+				   iface->conf->passive_scan_mode,
+				   iface->conf->passive_scan_interval,
+				   iface->conf->passive_scan_listen,
+				   NULL, NULL);
+	if (ret) {
+		if (ret == -1) {
+			wpa_printf(MSG_DEBUG, "Passive scanning not "
+				   "supported");
+		} else {
+			wpa_printf(MSG_ERROR, "Could not set passive "
+				   "scanning: %s", strerror(ret));
+		}
+		ret = 0;
+	}
 
-/**
- * hostapd_auto_chan_select_stop - Stops automatic channel selection
- * @iface: Pointer to interface data.
- * Returns:  0 if successfully stopped;
- *          -1 on failure (i.e., was not in progress)
- */
-int hostapd_select_hw_mode_stop(struct hostapd_iface *iface)
-{
-	return select_hw_mode_finalize(iface, -1);
+	return ret;
 }
 
 

hostapd/hw_features.h

@@ -52,9 +52,7 @@ struct hostapd_hw_modes {
 void hostapd_free_hw_features(struct hostapd_hw_modes *hw_features,
 			      size_t num_hw_features);
 int hostapd_get_hw_features(struct hostapd_iface *iface);
-int hostapd_select_hw_mode_start(struct hostapd_iface *iface,
-				 hostapd_iface_cb cb);
-int hostapd_select_hw_mode_stop(struct hostapd_iface *iface);
+int hostapd_select_hw_mode(struct hostapd_iface *iface);
 const char * hostapd_hw_mode_txt(int mode);
 int hostapd_hw_get_freq(struct hostapd_data *hapd, int chan);
 int hostapd_hw_get_channel(struct hostapd_data *hapd, int freq);