On Friday 02 May 2008, Glenn Saberton wrote: > -int reconfigure_wifi (struct debconfclient *client) > +int reconfigure_wifi(struct debconfclient *client) > { > - enum { ABORT, DONE, ESSID, WEP } wifistate = ESSID; > - for (;;) { > - switch (wifistate) { > - case ESSID: > - wifistate = ( netcfg_wireless_set_essid(client, interface, > "high") == GO_BACK ) ? - ABORT : WEP; > - break; > - case WEP: > - wifistate = ( netcfg_wireless_set_wep (client, interface) == > GO_BACK ) ? - ESSID : DONE; > - break; > - case ABORT: > - return REPLY_ASK_OPTIONS; > - break; > - case DONE: > - return REPLY_CHECK_DHCP; > - break; > - } > + > + enum { ABORT, ESSID, SECURITY, WEP, WPA, DONE } wifistate = ESSID; > + > + kill_wpa_supplicant(); > + kill_dhcp_client();
Looks like this change (moving kill_dhcp_client here) should have been included in the 2nd patch. > + for (;;) { > + switch (wifistate) { > + case ESSID: > + wifistate = ( netcfg_wireless_set_essid > (client, interface, "high") == GO_BACK ) ? + > ABORT : SECURITY; > + break; > + case SECURITY: > + { > + int ret; > + ret = netcfg_wireless_set_security (client, > interface); + if (ret == GO_BACK) > + wifistate = ESSID; > + else if (ret == REPLY_WPA) > + wifistate = WPA; > + else > + wifistate = WEP; > + break; > + } > + case WEP: > + wifistate = (netcfg_wireless_set_wep > (client, interface) == GO_BACK) ? + > SECURITY : DONE; > + break; > + case WPA: > + wifistate = (netcfg_set_passphrase (client, > interface) == GO_BACK) ? + SECURITY : > DONE; > + break; > + case DONE: > + return 0; > + break; > + case ABORT: > + return 1; > + break; > + } > } > } Why are you changing indentation here? It makes the actual changes much harder to see. > int netcfg_activate_dhcp (struct debconfclient *client) > { > char* dhostname = NULL; > @@ -496,15 +523,10 @@ int netcfg_activate_dhcp (struct debconfclient > *client) } > break; > case REPLY_RECONFIGURE_WIFI: > - if (reconfigure_wifi(client) == REPLY_CHECK_DHCP) > - if (dhcp_pid > 0) > - state = POLL; > - else { > - kill_dhcp_client(); > - state = START; > - } > - else > - state = ASK_OPTIONS; > + if (!reconfigure_wifi(client)) > + state = START; > + else > + state = ASK_OPTIONS; > break; > } Again, it looks to me like these changes could also have been done in the 2nd preparatory patch. And you're also changing indentation again. > --- a/packages/netcfg/netcfg-static.c > +++ b/packages/netcfg/netcfg-static.c > @@ -32,16 +32,19 @@ int main(int argc, char** argv) > int num_interfaces = 0; > static struct debconfclient *client; > static int requested_wireless_tools = 0; > + int requested_wpa_supplicant = 0; > > enum { BACKUP, > - GET_INTERFACE, > - GET_HOSTNAME_ONLY, > - GET_STATIC, > - WCONFIG, > - WCONFIG_ESSID, > - WCONFIG_WEP, > - QUIT } > - > + GET_INTERFACE, > + GET_HOSTNAME_ONLY, > + GET_STATIC, > + WCONFIG, > + WCONFIG_ESSID, > + WCONFIG_SECURITY, > + WCONFIG_WEP, > + WCONFIG_WPA, > + QUIT } > + > state = GET_INTERFACE; Random change of indentation making changes harder to see. > @@ -109,7 +122,19 @@ int main(int argc, char** argv) > else > state = GET_STATIC; > break; > - > + Random whitespace change. > --- a/packages/netcfg/netcfg.c > +++ b/packages/netcfg/netcfg.c > @@ -61,19 +62,22 @@ response_t netcfg_get_method(struct debconfclient > *client) int main(int argc, char *argv[]) > { > int num_interfaces = 0; > - enum { BACKUP, > - GET_INTERFACE, > - GET_HOSTNAME_ONLY, > - GET_METHOD, > - GET_DHCP, > - GET_STATIC, > - WCONFIG, > - WCONFIG_ESSID, > - WCONFIG_WEP, > - QUIT } > - > + > + enum { BACKUP, > + GET_INTERFACE, > + GET_HOSTNAME_ONLY, > + GET_METHOD, > + GET_DHCP, > + GET_STATIC, > + WCONFIG, > + WCONFIG_ESSID, > + WCONFIG_SECURITY, > + WCONFIG_WEP, > + WCONFIG_WPA, > + QUIT } > + > state = GET_INTERFACE; > - > + Random change of indentation and whitespace change. > @@ -249,23 +253,49 @@ int main(int argc, char *argv[]) > } > state = WCONFIG_ESSID; > break; > - > + Random whitespace change. > case WCONFIG_WEP: > - if (netcfg_wireless_set_wep (client, interface) == GO_BACK) > - state = WCONFIG_ESSID; > + if (netcfg_wireless_set_wep(client, interface) == GO_BACK) > + state = WCONFIG_SECURITY; > else > state = GET_METHOD; > break; > - > + Random whitespace change.
signature.asc
Description: This is a digitally signed message part.