lkml.org 
[lkml]   [2010]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Staging: wlan-ng: p80211netdev.c cleanup


    Nils Radtke schrieb:
    > Effected preliminary cleanup lead by the idea of kerneljanitor.org .
    > As recommended I'm asking for approval of the location of cleanup
    > and the manner it happened to know I'm heading in the right
    > direction.
    >
    > Compiles. Not tested.
    >
    > Signed-off-by: Nils Radtke <lkml@Think-Future.com>
    > ---
    > drivers/staging/wlan-ng/p80211netdev.c | 50 +++++++++++++++++++-------------
    > 1 files changed, 30 insertions(+), 20 deletions(-)
    >
    > diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c
    > index aa1792c..8104e11 100644
    > --- a/drivers/staging/wlan-ng/p80211netdev.c
    > +++ b/drivers/staging/wlan-ng/p80211netdev.c
    > @@ -176,25 +176,30 @@ static struct net_device_stats *p80211knetdev_get_stats(netdevice_t * netdev)
    > ----------------------------------------------------------------*/
    > static int p80211knetdev_open(netdevice_t *netdev)
    > {
    > - int result = 0; /* success */
    > + int ret = 0;
    > wlandevice_t *wlandev = netdev->ml_priv;
    >
    > /* Check to make sure the MSD is running */
    > - if (wlandev->msdstate != WLAN_MSD_RUNNING)
    > - return -ENODEV;
    > + if (wlandev->msdstate != WLAN_MSD_RUNNING) {
    > + ret = -ENODEV;
    > + goto end;
    > + }
    >
    > /* Tell the MSD to open */
    > - if (wlandev->open != NULL) {
    > - result = wlandev->open(wlandev);
    > - if (result == 0) {
    > - netif_start_queue(wlandev->netdev);
    > - wlandev->state = WLAN_DEVICE_OPEN;
    > - }
    > - } else {
    > - result = -EAGAIN;
    > + if (wlandev->open == NULL) {
    > + printk(KERN_ERR "Sorry, got wlandev->open == NULL.\n");
    > + ret = -EAGAIN;
    > + goto end;
    > }
    >
    > - return result;
    > + ret = wlandev->open(wlandev);
    > + if (ret) {
    > + netif_start_queue(wlandev->netdev);
    > + wlandev->state = WLAN_DEVICE_OPEN;
    > + }
    > +
    > +end:
    > + return ret;
    > }
    >

    unwinding is always good :)





    > /*----------------------------------------------------------------
    > @@ -211,16 +216,23 @@ static int p80211knetdev_open(netdevice_t *netdev)
    > ----------------------------------------------------------------*/
    > static int p80211knetdev_stop(netdevice_t *netdev)
    > {
    > - int result = 0;
    > + int ret = -EFAULT;
    > wlandevice_t *wlandev = netdev->ml_priv;
    >
    > - if (wlandev->close != NULL)
    > - result = wlandev->close(wlandev);
    > + if (wlandev->close == NULL) {
    > + printk(KERN_ERR "Sorry, got wlandev->close == NULL.\n");
    > + ret = -EFAULT; /* FIXME: nr: correct return code? */
    > + goto end;
    > + }
    >
    > - netif_stop_queue(wlandev->netdev);
    > - wlandev->state = WLAN_DEVICE_CLOSED;
    > + ret = wlandev->close(wlandev);
    > + if (ret) {
    > + netif_stop_queue(wlandev->netdev);
    > + wlandev->state = WLAN_DEVICE_CLOSED;
    > + }
    >
    > - return result;
    > +end:
    > + return ret;
    > }
    >

    but i wonder if the the test for wlandev->open in X_close() is needed
    can wlandev->close vanish between open() and stop() ?

    I am wondering who to use the error message "Sorry, got wlandev->open == NULL"
    is true but what help ? Give the reader a change and add __func__ to figure out
    what hit him.

    re,
    wh


    > /*----------------------------------------------------------------
    > @@ -242,8 +254,6 @@ void p80211netdev_rx(wlandevice_t *wlandev, struct sk_buff *skb)
    > skb_queue_tail(&wlandev->nsd_rxq, skb);
    >
    > tasklet_schedule(&wlandev->rx_bh);
    > -
    > - return;
    > }
    >
    > /*----------------------------------------------------------------


    \
     
     \ /
      Last update: 2010-09-19 14:43    [W:0.028 / U:1.608 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site