[PATCH] libertas: fix error handling
Dan Williams
dcbw at redhat.com
Sun Feb 25 08:44:22 EST 2007
On Sat, 2007-02-24 at 16:58 +0100, Holger Schurig wrote:
> * renamed wlan_add/remove_XXX into libertas_add/remove_XXX
> * change libertas_remove_card() so that it can clean up even
> half-initialized netdev/adapter/wlan_private objects
> * add some doxygen comments about this
>
> Signed-off-by: Holger Schurig <hs4233 at mail.mn-solutions.de>
> ---
> drivers/net/wireless/libertas/decl.h | 8 +-
> drivers/net/wireless/libertas/if_usb.c | 22 +++---
> drivers/net/wireless/libertas/main.c | 117 +++++++++++++++++--------------
> 3 files changed, 80 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/decl.h b/drivers/net/wireless/libertas/decl.h
> index 5d45a5c..d7bb7b0 100644
> --- a/drivers/net/wireless/libertas/decl.h
> +++ b/drivers/net/wireless/libertas/decl.h
> @@ -76,10 +76,10 @@ void libertas_send_iwevcustom_event(wlan_private * priv, s8 * str);
> /* main.c */
> extern struct chan_freq_power *libertas_get_region_cfp_table(u8 region, u8 band,
> int *cfp_no);
> -wlan_private *wlan_add_card(void *card);
> +wlan_private *libertas_add_card(void *card);
> int libertas_activate_card(wlan_private *priv);
> -int wlan_remove_card(wlan_private *priv);
> -int wlan_add_mesh(wlan_private *priv);
> -void wlan_remove_mesh(wlan_private *priv);
> +int libertas_remove_card(wlan_private *priv);
> +int libertas_add_mesh(wlan_private *priv);
> +void libertas_remove_mesh(wlan_private *priv);
>
> #endif /* _WLAN_DECL_H_ */
> diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
> index 5c06f0f..c48e09f 100644
> --- a/drivers/net/wireless/libertas/if_usb.c
> +++ b/drivers/net/wireless/libertas/if_usb.c
> @@ -104,7 +104,7 @@ static int if_usb_probe(struct usb_interface *intf,
> struct usb_device *udev;
> struct usb_host_interface *iface_desc;
> struct usb_endpoint_descriptor *endpoint;
> - wlan_private *priv;
> + wlan_private *priv = NULL;
> struct usb_card_rec *usb_cardp;
> int i;
>
> @@ -188,11 +188,11 @@ static int if_usb_probe(struct usb_interface *intf,
> }
>
>
> - /* At this point wlan_add_card() will be called. Don't worry
> - * about keeping pwlanpriv around since it will be set on our
> + /* At this point libertas_add_card() will be called. Don't worry
> + * about keeping priv around since it will be set on our
> * usb device data in -> add() -> libertas_sbi_register_dev().
> */
> - if (!(priv = wlan_add_card(usb_cardp)))
> + if (!(priv = libertas_add_card(usb_cardp)))
> goto dealloc;
>
> if (libertas_activate_card(priv))
> @@ -203,7 +203,8 @@ static int if_usb_probe(struct usb_interface *intf,
> libertas_found++;
> }
>
> - if (wlan_add_mesh(priv))
> + /* TODO: check somehow if the firmware is mesh-capable */
> + if (libertas_add_mesh(priv))
> goto dealloc;
>
> usb_get_dev(udev);
> @@ -217,6 +218,8 @@ static int if_usb_probe(struct usb_interface *intf,
> return 0;
>
> dealloc:
> + libertas_remove_mesh(priv);
> + libertas_remove_card(priv);
> if_usb_free(usb_cardp);
>
> error:
> @@ -246,23 +249,21 @@ static void if_usb_disconnect(struct usb_interface *intf)
> for (i = 0; i<libertas_found; i++) {
> if (libertas_devs[i]==priv->wlan_dev.netdev) {
> libertas_devs[i] = libertas_devs[--libertas_found];
> - libertas_devs[libertas_found] = NULL ;
> + libertas_devs[libertas_found] = NULL;
> break;
> }
> }
>
> /* card is removed and we can call wlan_remove_card */
> lbs_deb_usbd(&cardp->udev->dev, "call remove card\n");
> - wlan_remove_mesh(priv);
> - wlan_remove_card(priv);
> + libertas_remove_mesh(priv);
> + libertas_remove_card(priv);
>
> /* Unlink and free urb */
> if_usb_free(cardp);
>
> usb_set_intfdata(intf, NULL);
> usb_put_dev(interface_to_usbdev(intf));
> -
> - return;
> }
>
> /**
> @@ -750,6 +751,7 @@ static int reset_device(wlan_private *priv)
> int ret;
>
> lbs_deb_enter(LBS_DEB_USB);
> +
> ret = libertas_prepare_and_send_command(priv, cmd_802_11_reset,
> cmd_act_halt, 0, 0, NULL);
> msleep_interruptible(10);
> diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
> index 507c4f4..25efdda 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -751,10 +751,14 @@ static int wlan_service_main_thread(void *data)
> * @brief This function adds the card. it will probe the
> * card, allocate the wlan_priv and initialize the device.
> *
> - * @param card A pointer to card
> - * @return A pointer to wlan_private structure
> + * Our caller is responsible to call libertas_remove_card() if
> + * we return an error condition!
> + *
> + * @param card A pointer to card
> + * @return A pointer to wlan_private structure or
> + * NULL in case of an error
> */
> -wlan_private *wlan_add_card(void *card)
> +wlan_private *libertas_add_card(void *card)
> {
> struct net_device *dev = NULL;
> wlan_private *priv = NULL;
> @@ -771,7 +775,7 @@ wlan_private *wlan_add_card(void *card)
> /* allocate buffer for wlan_adapter */
> if (!(priv->adapter = kzalloc(sizeof(wlan_adapter), GFP_KERNEL))) {
> lbs_pr_err("allocate buffer for wlan_adapter failed\n");
> - goto err_kzalloc;
> + goto done;
> }
>
> priv->wlan_dev.netdev = dev;
> @@ -807,13 +811,23 @@ wlan_private *wlan_add_card(void *card)
> priv->adapter->nr_cmd_pending = 0;
> goto done;
>
> -err_kzalloc:
> - free_netdev(dev);
> done:
> lbs_deb_leave_args(LBS_DEB_NET, "priv %p", priv);
> return priv;
> }
>
> +/**
> + * @brief This function starts the thread and other logic
> + * to activate the card. This can happen after our called
> + * called libertas_add_card() and filled in the priv->hw_XXXX
> + * function pointers.
> + *
> + * Our caller is also responsible to call libertas_remove_card() if
> + * we return an error condition!
> + *
> + * @param card A pointer to card
> + * @return 0 if successful, -X otherwise
> + */
> int libertas_activate_card(wlan_private *priv)
> {
> struct net_device *dev = priv->wlan_dev.netdev;
> @@ -874,21 +888,23 @@ done:
>
> /**
> * @brief This function adds mshX interface
> + *
> + * Our caller is responsible to call libertas_remove_mesh() if
> + * we return an error condition!
> *
> - * @param priv A pointer to the wlan_private structure
> - * @return 0 if successful, -X otherwise
> + * @param priv A pointer to the wlan_private structure
> + * @return 0 if successful, -X otherwise
> */
> -int wlan_add_mesh(wlan_private *priv)
> +int libertas_add_mesh(wlan_private *priv)
> {
> struct net_device *mesh_dev = NULL;
> - int ret = 0;
> + int ret = -1;
>
> lbs_deb_enter(LBS_DEB_MESH);
>
> /* Allocate a virtual mesh device */
> if (!(mesh_dev = alloc_netdev(0, "msh%d", ether_setup))) {
> lbs_deb_mesh("init mshX device failed\n");
> - ret = -ENOMEM;
> goto done;
> }
> mesh_dev->priv = priv;
> @@ -914,24 +930,16 @@ int wlan_add_mesh(wlan_private *priv)
> ret = register_netdev(mesh_dev);
> if (ret) {
> lbs_pr_err("cannot register mshX virtual interface\n");
> - goto err_free;
> + goto done;
> }
>
> ret = class_device_create_file(&(mesh_dev->class_dev), &class_device_attr_libertas_mpp);
> if (ret)
> - goto err_unregister;
> + goto done;
>
> /* Everything successful */
> ret = 0;
> - goto done;
>
> -
> -err_unregister:
> - unregister_netdev(mesh_dev);
> -
> -err_free:
> - free_netdev(mesh_dev);
> -
> done:
> lbs_deb_leave_args(LBS_DEB_MESH, "ret %d", ret);
> return ret;
> @@ -953,10 +961,10 @@ static void wake_pending_cmdnodes(wlan_private *priv)
> }
>
>
> -int wlan_remove_card(wlan_private *priv)
> +int libertas_remove_card(wlan_private *priv)
> {
> wlan_adapter *adapter;
> - struct net_device *dev;
> + struct net_device *dev = NULL;
> union iwreq_data wrqu;
>
> lbs_deb_enter(LBS_DEB_NET);
> @@ -965,52 +973,54 @@ int wlan_remove_card(wlan_private *priv)
> goto out;
>
> adapter = priv->adapter;
> -
> - if (!adapter)
> - goto out;
> -
> dev = priv->wlan_dev.netdev;
>
> - netif_stop_queue(priv->wlan_dev.netdev);
> - netif_carrier_off(priv->wlan_dev.netdev);
> + if (adapter) {
> + if (dev) {
> + netif_stop_queue(dev);
> + netif_carrier_off(dev);
> + }
>
> - wake_pending_cmdnodes(priv);
> + wake_pending_cmdnodes(priv);
>
> - unregister_netdev(dev);
> + if (dev && dev->reg_state)
> + unregister_netdev(dev);
>
> - cancel_delayed_work(&priv->assoc_work);
> - destroy_workqueue(priv->assoc_thread);
> + cancel_delayed_work(&priv->assoc_work);
> + if (priv->assoc_thread)
> + destroy_workqueue(priv->assoc_thread);
>
> - if (adapter->psmode == wlan802_11powermodemax_psp) {
> - adapter->psmode = wlan802_11powermodecam;
> - libertas_ps_wakeup(priv, cmd_option_waitforrsp);
> - }
> + if (adapter->psmode == wlan802_11powermodemax_psp) {
> + adapter->psmode = wlan802_11powermodecam;
> + libertas_ps_wakeup(priv, cmd_option_waitforrsp);
> + }
>
> - memset(wrqu.ap_addr.sa_data, 0xaa, ETH_ALEN);
> - wrqu.ap_addr.sa_family = ARPHRD_ETHER;
> - wireless_send_event(priv->wlan_dev.netdev, SIOCGIWAP, &wrqu, NULL);
> + memset(wrqu.ap_addr.sa_data, 0xaa, ETH_ALEN);
Another commit should change this 0xaa to 0x00; I know you didn't write
this bit (you just touched it) but this part is technically invalid for
the WEXT "spec". Only 0x00 is a valid "I'm no longer associated"
message for SIOCGIWAP.
Patch looks good though.
Dan
> + wrqu.ap_addr.sa_family = ARPHRD_ETHER;
> + wireless_send_event(dev, SIOCGIWAP, &wrqu, NULL);
>
> - adapter->surpriseremoved = 1;
> + adapter->surpriseremoved = 1;
>
> - /* Stop the thread servicing the interrupts */
> - wlan_terminate_thread(&priv->mainthread);
> + /* Stop the thread servicing the interrupts */
> + if (priv->mainthread.task)
> + wlan_terminate_thread(&priv->mainthread);
>
> - libertas_debugfs_remove_one(priv);
> + libertas_free_adapter(priv);
> + }
>
> - lbs_deb_net("free adapter\n");
> - libertas_free_adapter(priv);
> -
> - lbs_deb_net("unregister finish\n");
> + libertas_debugfs_remove_one(priv);
>
> - priv->wlan_dev.netdev = NULL;
> - free_netdev(dev);
> + if (dev) {
> + priv->wlan_dev.netdev = NULL;
> + free_netdev(dev);
> + }
>
> out:
> lbs_deb_leave(LBS_DEB_NET);
> return 0;
> }
>
> -void wlan_remove_mesh(wlan_private *priv)
> +void libertas_remove_mesh(wlan_private *priv)
> {
> struct net_device *mesh_dev;
>
> @@ -1020,13 +1030,14 @@ void wlan_remove_mesh(wlan_private *priv)
> goto out;
>
> mesh_dev = priv->mesh_dev;
> + if (!mesh_dev)
> + goto out;
>
> netif_stop_queue(mesh_dev);
>
> class_device_remove_file(&(mesh_dev->class_dev), &class_device_attr_libertas_mpp);
> unregister_netdev(mesh_dev);
> -
> - priv->mesh_dev = NULL ;
> + priv->mesh_dev = NULL;
> free_netdev(mesh_dev);
>
> out:
More information about the libertas-dev
mailing list