lkml.org 
[lkml]   [2011]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 04/12] staging: usbip: stub_main.c: code cleanup
    On 11:08 Fri 20 May     , walter harms wrote:
    >
    >
    > Am 20.05.2011 06:36, schrieb matt mooney:
    > > Remove match_find() and replace with get_busid_idx(); change
    > > get_busid_priv(), add_match_busid(), and del_match_busid() to use
    > > get_busid_idx(); and cleanup code in the other functions.
    > >
    > > Signed-off-by: matt mooney <mfm@muteddisk.com>
    > > ---
    > > drivers/staging/usbip/stub_main.c | 147 ++++++++++++++++++-------------------
    > > 1 files changed, 73 insertions(+), 74 deletions(-)
    > >
    > > diff --git a/drivers/staging/usbip/stub_main.c b/drivers/staging/usbip/stub_main.c
    > > index 0ca1462..00398a6 100644
    > > --- a/drivers/staging/usbip/stub_main.c
    > > +++ b/drivers/staging/usbip/stub_main.c
    > > @@ -50,82 +50,90 @@ static void init_busid_table(void)
    > > spin_lock_init(&busid_table_lock);
    > > }
    > >
    > > -int match_busid(const char *busid)
    > > +/*
    > > + * Find the index of the busid by name.
    > > + * Must be called with busid_table_lock held.
    > > + */
    > > +static int get_busid_idx(const char *busid)
    > > {
    > > int i;
    > > + int idx = -1;
    > >
    > > - spin_lock(&busid_table_lock);
    > > for (i = 0; i < MAX_BUSID; i++)
    > > if (busid_table[i].name[0])
    > > if (!strncmp(busid_table[i].name, busid, BUSID_SIZE)) {
    > > - /* already registerd */
    > > - spin_unlock(&busid_table_lock);
    > > - return 0;
    > > + idx = i;
    > > + break;
    > > }
    > > - spin_unlock(&busid_table_lock);
    > > -
    > > - return 1;
    > > + return idx;
    > > }
    > >
    > > struct bus_id_priv *get_busid_priv(const char *busid)
    > > {
    > > - int i;
    > > + int idx;
    > > + struct bus_id_priv *bid = NULL;
    > >
    > > spin_lock(&busid_table_lock);
    > > - for (i = 0; i < MAX_BUSID; i++)
    > > - if (busid_table[i].name[0])
    > > - if (!strncmp(busid_table[i].name, busid, BUSID_SIZE)) {
    > > - /* already registerd */
    > > - spin_unlock(&busid_table_lock);
    > > - return &(busid_table[i]);
    > > - }
    > > + idx = get_busid_idx(busid);
    > > + if (idx >= 0)
    > > + bid = &(busid_table[idx]);
    > > spin_unlock(&busid_table_lock);
    > >
    > > - return NULL;
    > > + return bid;
    > > }
    > >
    > > static int add_match_busid(char *busid)
    > > {
    > > int i;
    > > -
    > > - if (!match_busid(busid))
    > > - return 0;
    > > + int ret = -1;
    > >
    > > spin_lock(&busid_table_lock);
    > > + /* already registered? */
    > > + if (get_busid_idx(busid) >= 0) {
    > > + ret = 0;
    > > + goto out;
    > > + }
    > > +
    > > for (i = 0; i < MAX_BUSID; i++)
    > > if (!busid_table[i].name[0]) {
    > > strncpy(busid_table[i].name, busid, BUSID_SIZE);
    >
    > i am missing an if() here ??

    I am not sure what you mean. It should be correct.

    >
    > > if ((busid_table[i].status != STUB_BUSID_ALLOC) &&
    > > (busid_table[i].status != STUB_BUSID_REMOV))
    > > busid_table[i].status = STUB_BUSID_ADDED;
    > > - spin_unlock(&busid_table_lock);
    > > - return 0;
    > > + ret = 0;
    > > + break;
    > > }
    > > +
    >
    >
    >
    >
    >
    > > +out:
    > > spin_unlock(&busid_table_lock);
    > >
    > > - return -1;
    > > + return ret;
    > > }
    > >
    > > int del_match_busid(char *busid)
    > > {
    > > - int i;
    > > + int idx;
    > > + int ret = -1;
    > >
    > > spin_lock(&busid_table_lock);
    > > - for (i = 0; i < MAX_BUSID; i++)
    > > - if (!strncmp(busid_table[i].name, busid, BUSID_SIZE)) {
    > > - /* found */
    > > - if (busid_table[i].status == STUB_BUSID_OTHER)
    > > - memset(busid_table[i].name, 0, BUSID_SIZE);
    > > - if ((busid_table[i].status != STUB_BUSID_OTHER) &&
    > > - (busid_table[i].status != STUB_BUSID_ADDED)) {
    > > - busid_table[i].status = STUB_BUSID_REMOV;
    > > - }
    > > - spin_unlock(&busid_table_lock);
    > > - return 0;
    > > - }
    > > + idx = get_busid_idx(busid);
    > > + if (idx < 0)
    > > + goto out;
    > > +
    > > + /* found */
    > > + ret = 0;
    > > +
    > > + if (busid_table[idx].status == STUB_BUSID_OTHER)
    > > + memset(busid_table[idx].name, 0, BUSID_SIZE);
    > > +
    > > + if ((busid_table[idx].status != STUB_BUSID_OTHER) &&
    > > + (busid_table[idx].status != STUB_BUSID_ADDED))
    > > + busid_table[idx].status = STUB_BUSID_REMOV;
    > > +
    > > +out:
    > > spin_unlock(&busid_table_lock);
    > >
    > > - return -1;
    > > + return ret;
    > > }
    > >
    > > static ssize_t show_match_busid(struct device_driver *drv, char *buf)
    > > @@ -138,8 +146,8 @@ static ssize_t show_match_busid(struct device_driver *drv, char *buf)
    > > if (busid_table[i].name[0])
    > > out += sprintf(out, "%s ", busid_table[i].name);
    > > spin_unlock(&busid_table_lock);
    > > -
    > > out += sprintf(out, "\n");
    > > +
    > > return out - buf;
    > > }
    > >
    > > @@ -162,23 +170,24 @@ static ssize_t store_match_busid(struct device_driver *dev, const char *buf,
    > > strncpy(busid, buf + 4, BUSID_SIZE);
    > >
    > > if (!strncmp(buf, "add ", 4)) {
    > > - if (add_match_busid(busid) < 0)
    > > + if (add_match_busid(busid) < 0) {
    > > return -ENOMEM;
    > > - else {
    > > + } else {
    > > pr_debug("add busid %s\n", busid);
    > > return count;
    > > }
    > > } else if (!strncmp(buf, "del ", 4)) {
    > > - if (del_match_busid(busid) < 0)
    > > + if (del_match_busid(busid) < 0) {
    > > return -ENODEV;
    > > - else {
    > > + } else {
    > > pr_debug("del busid %s\n", busid);
    > > return count;
    > > }
    > > - } else
    > > + } else {
    > > return -EINVAL;
    > > + }
    > > }
    > > -static DRIVER_ATTR(match_busid, S_IRUSR|S_IWUSR, show_match_busid,
    > > +static DRIVER_ATTR(match_busid, S_IRUSR | S_IWUSR, show_match_busid,
    > > store_match_busid);
    > >
    > > static struct stub_priv *stub_priv_pop_from_listhead(struct list_head *listhead)
    > > @@ -201,36 +210,30 @@ static struct stub_priv *stub_priv_pop(struct stub_device *sdev)
    > > spin_lock_irqsave(&sdev->priv_lock, flags);
    > >
    > > priv = stub_priv_pop_from_listhead(&sdev->priv_init);
    > > - if (priv) {
    > > - spin_unlock_irqrestore(&sdev->priv_lock, flags);
    > > - return priv;
    > > - }
    > > + if (priv)
    > > + goto done;
    > >
    > > priv = stub_priv_pop_from_listhead(&sdev->priv_tx);
    > > - if (priv) {
    > > - spin_unlock_irqrestore(&sdev->priv_lock, flags);
    > > - return priv;
    > > - }
    > > + if (priv)
    > > + goto done;
    > >
    > > priv = stub_priv_pop_from_listhead(&sdev->priv_free);
    > > - if (priv) {
    > > - spin_unlock_irqrestore(&sdev->priv_lock, flags);
    > > - return priv;
    > > - }
    > >
    > > +done:
    > > spin_unlock_irqrestore(&sdev->priv_lock, flags);
    > > - return NULL;
    > > +
    > > + return priv;
    > > }
    > >
    > > void stub_device_cleanup_urbs(struct stub_device *sdev)
    > > {
    > > struct stub_priv *priv;
    > > + struct urb *urb;
    > >
    > > dev_dbg(&sdev->udev->dev, "free sdev %p\n", sdev);
    > >
    > > while ((priv = stub_priv_pop(sdev))) {
    > > - struct urb *urb = priv->urb;
    > > -
    > > + urb = priv->urb;
    > > dev_dbg(&sdev->udev->dev, "free urb %p\n", urb);
    > > usb_kill_urb(urb);
    > >
    > > @@ -238,7 +241,6 @@ void stub_device_cleanup_urbs(struct stub_device *sdev)
    > >
    > > kfree(urb->transfer_buffer);
    > > kfree(urb->setup_packet);
    > > -
    > > usb_free_urb(urb);
    > > }
    > > }
    > > @@ -250,34 +252,31 @@ static int __init usb_stub_init(void)
    > > stub_priv_cache = kmem_cache_create("stub_priv",
    > > sizeof(struct stub_priv), 0,
    > > SLAB_HWCACHE_ALIGN, NULL);
    > > -
    > > if (!stub_priv_cache) {
    > > - pr_err("create stub_priv_cache error\n");
    > > + pr_err("kmem_cache_create failed\n");
    > > return -ENOMEM;
    > > }
    > >
    > > ret = usb_register(&stub_driver);
    > > - if (ret) {
    > > + if (ret < 0) {
    > > pr_err("usb_register failed %d\n", ret);
    > > - goto error_usb_register;
    > > + goto err_usb_register;
    > > }
    > >
    > > - pr_info(DRIVER_DESC " " USBIP_VERSION "\n");
    > > -
    > > - init_busid_table();
    > > -
    > > ret = driver_create_file(&stub_driver.drvwrap.driver,
    > > &driver_attr_match_busid);
    > > -
    > > - if (ret) {
    > > - pr_err("create driver sysfs\n");
    > > - goto error_create_file;
    > > + if (ret < 0) {
    > > + pr_err("driver_create_file failed\n");
    > > + goto err_create_file;
    > > }
    > >
    > > + init_busid_table();
    > > + pr_info(DRIVER_DESC " v" USBIP_VERSION "\n");
    > > return ret;
    > > -error_create_file:
    > > +
    > > +err_create_file:
    > > usb_deregister(&stub_driver);
    > > -error_usb_register:
    > > +err_usb_register:
    > > kmem_cache_destroy(stub_priv_cache);
    > > return ret;
    > > }
    >


    \
     
     \ /
      Last update: 2011-05-20 20:55    [W:0.039 / U:38.428 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site