2010-06-27 13:21:16

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.

Change defined STATUS_XXX return codes to standard -EYYY.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/staging/slicoss/slicoss.c | 52 ++++++++++++++++++------------------
1 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index bebf0fd..102d3ea 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -452,7 +452,7 @@ static int __devinit slic_entry_probe(struct pci_dev *pcidev,

status = slic_card_init(card, adapter);

- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
card->state = CARD_FAIL;
adapter->state = ADAPT_FAIL;
adapter->linkstate = LINK_DOWN;
@@ -513,7 +513,7 @@ static int slic_entry_open(struct net_device *dev)
}
status = slic_if_init(adapter);

- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
if (adapter->activated) {
card->adapters_activated--;
slic_global.num_slic_ports_active--;
@@ -535,7 +535,7 @@ static int slic_entry_open(struct net_device *dev)
locked = 0;
}

- return STATUS_SUCCESS;
+ return 0;
}

static void __devexit slic_entry_remove(struct pci_dev *pcidev)
@@ -628,7 +628,7 @@ static int slic_entry_halt(struct net_device *dev)

spin_unlock_irqrestore(&slic_global.driver_lock.lock,
slic_global.driver_lock.flags);
- return STATUS_SUCCESS;
+ return 0;
}

static int slic_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
@@ -1206,7 +1206,7 @@ static void slic_link_event_handler(struct adapter *adapter)
#else
Stop compilation;
#endif
- ASSERT((status == STATUS_SUCCESS) || (status == STATUS_PENDING));
+ ASSERT(status == 0);
}

static void slic_init_cleanup(struct adapter *adapter)
@@ -1265,7 +1265,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address)
mlist = adapter->mcastaddrs;
while (mlist) {
if (!compare_ether_addr(mlist->address, address))
- return STATUS_SUCCESS;
+ return 0;
mlist = mlist->next;
}

@@ -1279,7 +1279,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address)
mcaddr->next = adapter->mcastaddrs;
adapter->mcastaddrs = mcaddr;

- return STATUS_SUCCESS;
+ return 0;
}

/*
@@ -1365,7 +1365,7 @@ static void slic_mcast_set_bit(struct adapter *adapter, char *address)
static void slic_mcast_set_list(struct net_device *dev)
{
struct adapter *adapter = netdev_priv(dev);
- int status = STATUS_SUCCESS;
+ int status = 0;
char *addresses;
struct netdev_hw_addr *ha;

@@ -1374,7 +1374,7 @@ static void slic_mcast_set_list(struct net_device *dev)
netdev_for_each_mc_addr(ha, dev) {
addresses = (char *) &ha->addr;
status = slic_mcast_add_list(adapter, addresses);
- if (status != STATUS_SUCCESS)
+ if (status != 0)
break;
slic_mcast_set_bit(adapter, addresses);
}
@@ -1394,7 +1394,7 @@ static void slic_mcast_set_list(struct net_device *dev)
adapter->devflags_prev = dev->flags;
slic_config_set(adapter, true);
} else {
- if (status == STATUS_SUCCESS)
+ if (status == 0)
slic_mcast_set_mask(adapter);
}
return;
@@ -1476,7 +1476,7 @@ static int slic_if_init(struct adapter *adapter)
adapter->macopts |= MAC_MCAST;
}
status = slic_adapter_allocresources(adapter);
- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
dev_err(&dev->dev,
"%s: slic_adapter_allocresources FAILED %x\n",
__func__, status);
@@ -1553,7 +1553,7 @@ static int slic_if_init(struct adapter *adapter)
slic_link_config(adapter, LINK_AUTOSPEED, LINK_AUTOD);
slic_link_event_handler(adapter);

- return STATUS_SUCCESS;
+ return 0;
}

static void slic_unmap_mmio_space(struct adapter *adapter)
@@ -1587,7 +1587,7 @@ static int slic_adapter_allocresources(struct adapter *adapter)
}
adapter->intrregistered = 1;
}
- return STATUS_SUCCESS;
+ return 0;
}

static void slic_config_pci(struct pci_dev *pcidev)
@@ -1967,7 +1967,7 @@ static int slic_card_download(struct adapter *adapter)
and reach mainloop */
mdelay(20);

- return STATUS_SUCCESS;
+ return 0;
}

MODULE_FIRMWARE("slicoss/oasisdownload.sys");
@@ -2027,7 +2027,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
/* Download the microcode */
status = slic_card_download(adapter);

- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
dev_err(&adapter->pcidev->dev,
"download failed bus %d slot %d\n",
adapter->busnumber, adapter->slotnumber);
@@ -2203,7 +2203,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
card->state = CARD_UP;
card->reset_in_progress = 0;

- return STATUS_SUCCESS;
+ return 0;
}

static u32 slic_card_locate(struct adapter *adapter)
@@ -2267,7 +2267,7 @@ static u32 slic_card_locate(struct adapter *adapter)

ASSERT(card);
if (!card)
- return STATUS_FAILURE;
+ return -ENOMEM;
/* Put the adapter in the card's adapter list */
ASSERT(card->adapter[adapter->port] == NULL);
if (!card->adapter[adapter->port]) {
@@ -2637,7 +2637,7 @@ static int slic_upr_queue_request(struct adapter *adapter,
} else {
adapter->upr_list = upr;
}
- return STATUS_SUCCESS;
+ return 0;
}

static int slic_upr_request(struct adapter *adapter,
@@ -2653,7 +2653,7 @@ static int slic_upr_request(struct adapter *adapter,
upr_request,
upr_data,
upr_data_h, upr_buffer, upr_buffer_h);
- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
spin_unlock_irqrestore(&adapter->upr_lock.lock,
adapter->upr_lock.flags);
return status;
@@ -2661,7 +2661,7 @@ static int slic_upr_request(struct adapter *adapter,
slic_upr_start(adapter);
spin_unlock_irqrestore(&adapter->upr_lock.lock,
adapter->upr_lock.flags);
- return STATUS_PENDING;
+ return 0;
}

static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
@@ -3032,7 +3032,7 @@ static int slic_rspqueue_init(struct adapter *adapter)
dev_err(&adapter->pcidev->dev,
"pci_alloc_consistent failed\n");
slic_rspqueue_free(adapter);
- return STATUS_FAILURE;
+ return -ENOMEM;
}
#ifndef CONFIG_X86_64
ASSERT(((u32) rspq->vaddr[i] & 0xFFFFF000) ==
@@ -3056,7 +3056,7 @@ static int slic_rspqueue_init(struct adapter *adapter)
rspq->offset = 0;
rspq->pageindex = 0;
rspq->rspbuf = (struct slic_rspbuf *)rspq->vaddr[0];
- return STATUS_SUCCESS;
+ return 0;
}

static void slic_rspqueue_free(struct adapter *adapter)
@@ -3180,13 +3180,13 @@ static int slic_cmdq_init(struct adapter *adapter)
#endif
if (!pageaddr) {
slic_cmdq_free(adapter);
- return STATUS_FAILURE;
+ return -ENOMEM;
}
slic_cmdq_addcmdpage(adapter, pageaddr);
}
adapter->slic_handle_ix = 1;

- return STATUS_SUCCESS;
+ return 0;
}

static void slic_cmdq_free(struct adapter *adapter)
@@ -3407,9 +3407,9 @@ static int slic_rcvqueue_init(struct adapter *adapter)
}
if (rcvq->count < SLIC_RCVQ_MINENTRIES) {
slic_rcvqueue_free(adapter);
- return STATUS_FAILURE;
+ return -ENOMEM;
}
- return STATUS_SUCCESS;
+ return 0;
}

static void slic_rcvqueue_free(struct adapter *adapter)
--
1.7.0.4


2010-06-27 13:21:57

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch

Move NULL pointer assertion to else branch because it can
become NULL only here.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/staging/slicoss/slicoss.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 102d3ea..86bc733 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2263,11 +2263,11 @@ static u32 slic_card_locate(struct adapter *adapter)
break;
card = card->next;
}
+ ASSERT(card);
+ if (!card)
+ return -ENOMEM;
}

- ASSERT(card);
- if (!card)
- return -ENOMEM;
/* Put the adapter in the card's adapter list */
ASSERT(card->adapter[adapter->port] == NULL);
if (!card->adapter[adapter->port]) {
--
1.7.0.4

2010-06-27 13:22:17

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH 4/5] staging: slicoss: error handling with goto

This patch makes error handling more readable due to 'goto err' pattern.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/staging/slicoss/slicoss.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 86bc733..d8c952e 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2646,22 +2646,21 @@ static int slic_upr_request(struct adapter *adapter,
u32 upr_data_h,
u32 upr_buffer, u32 upr_buffer_h)
{
- int status;
+ int rc;

spin_lock_irqsave(&adapter->upr_lock.lock, adapter->upr_lock.flags);
- status = slic_upr_queue_request(adapter,
+ rc = slic_upr_queue_request(adapter,
upr_request,
upr_data,
upr_data_h, upr_buffer, upr_buffer_h);
- if (status != 0) {
- spin_unlock_irqrestore(&adapter->upr_lock.lock,
- adapter->upr_lock.flags);
- return status;
- }
+ if (rc)
+ goto err_unlock_irq;
+
slic_upr_start(adapter);
+err_unlock_irq:
spin_unlock_irqrestore(&adapter->upr_lock.lock,
adapter->upr_lock.flags);
- return 0;
+ return rc;
}

static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
--
1.7.0.4

2010-06-27 13:22:42

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH 5/5] staging: slicoss: error handling with goto

This patch makes error handling more readable due to 'goto err' pattern.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/staging/slicoss/slicoss.c | 28 +++++++++++++++-------------
1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index d8c952e..478ba3b 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -1451,7 +1451,7 @@ static int slic_if_init(struct adapter *adapter)
struct net_device *dev = adapter->netdev;
__iomem struct slic_regs *slic_regs = adapter->slic_regs;
struct slic_shmem *pshmem;
- int status = 0;
+ int rc;

ASSERT(card);

@@ -1459,7 +1459,8 @@ static int slic_if_init(struct adapter *adapter)
if (adapter->state != ADAPT_DOWN) {
dev_err(&dev->dev, "%s: adapter->state != ADAPT_DOWN\n",
__func__);
- return -EIO;
+ rc = -EIO;
+ goto err;
}
ASSERT(adapter->linkstate == LINK_DOWN);

@@ -1475,22 +1476,22 @@ static int slic_if_init(struct adapter *adapter)
if (dev->flags & IFF_MULTICAST)
adapter->macopts |= MAC_MCAST;
}
- status = slic_adapter_allocresources(adapter);
- if (status != 0) {
+ rc = slic_adapter_allocresources(adapter);
+ if (rc) {
dev_err(&dev->dev,
"%s: slic_adapter_allocresources FAILED %x\n",
- __func__, status);
+ __func__, rc);
slic_adapter_freeresources(adapter);
- return status;
+ goto err;
}

if (!adapter->queues_initialized) {
- if (slic_rspqueue_init(adapter))
- return -ENOMEM;
- if (slic_cmdq_init(adapter))
- return -ENOMEM;
- if (slic_rcvqueue_init(adapter))
- return -ENOMEM;
+ if ((rc = slic_rspqueue_init(adapter)))
+ goto err;
+ if ((rc = slic_cmdq_init(adapter)))
+ goto err;
+ if ((rc = slic_rcvqueue_init(adapter)))
+ goto err;
adapter->queues_initialized = 1;
}

@@ -1553,7 +1554,8 @@ static int slic_if_init(struct adapter *adapter)
slic_link_config(adapter, LINK_AUTOSPEED, LINK_AUTOD);
slic_link_event_handler(adapter);

- return 0;
+err:
+ return rc;
}

static void slic_unmap_mmio_space(struct adapter *adapter)
--
1.7.0.4

2010-06-28 10:17:39

by Denis Kirjanov

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.

On Sun, Jun 27, 2010 at 5:20 PM, Kulikov Vasiliy <[email protected]> wrote:
> Change defined STATUS_XXX return codes to standard -EYYY.
>
> Signed-off-by: Kulikov Vasiliy <[email protected]>
> ---
> ?drivers/staging/slicoss/slicoss.c | ? 52 ++++++++++++++++++------------------
> ?1 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> index bebf0fd..102d3ea 100644
> --- a/drivers/staging/slicoss/slicoss.c
> +++ b/drivers/staging/slicoss/slicoss.c
> @@ -452,7 +452,7 @@ static int __devinit slic_entry_probe(struct pci_dev *pcidev,
>
> ? ? ? ?status = slic_card_init(card, adapter);
>
> - ? ? ? if (status != STATUS_SUCCESS) {
> + ? ? ? if (status != 0) {
> ? ? ? ? ? ? ? ?card->state = CARD_FAIL;
> ? ? ? ? ? ? ? ?adapter->state = ADAPT_FAIL;
> ? ? ? ? ? ? ? ?adapter->linkstate = LINK_DOWN;

Can we really continue here?

> @@ -513,7 +513,7 @@ static int slic_entry_open(struct net_device *dev)
> ? ? ? ?}
> ? ? ? ?status = slic_if_init(adapter);
>
> - ? ? ? if (status != STATUS_SUCCESS) {
> + ? ? ? if (status != 0) {
> ? ? ? ? ? ? ? ?if (adapter->activated) {
> ? ? ? ? ? ? ? ? ? ? ? ?card->adapters_activated--;
> ? ? ? ? ? ? ? ? ? ? ? ?slic_global.num_slic_ports_active--;
> @@ -535,7 +535,7 @@ static int slic_entry_open(struct net_device *dev)
> ? ? ? ? ? ? ? ?locked = 0;
> ? ? ? ?}
>
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static void __devexit slic_entry_remove(struct pci_dev *pcidev)
> @@ -628,7 +628,7 @@ static int slic_entry_halt(struct net_device *dev)
>
> ? ? ? ?spin_unlock_irqrestore(&slic_global.driver_lock.lock,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?slic_global.driver_lock.flags);
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static int slic_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> @@ -1206,7 +1206,7 @@ static void slic_link_event_handler(struct adapter *adapter)
> ?#else
> ? ? ? ?Stop compilation;
> ?#endif
> - ? ? ? ASSERT((status == STATUS_SUCCESS) || (status == STATUS_PENDING));
> + ? ? ? ASSERT(status == 0);
> ?}
>

Now that looks useless since slic_upr_request can return STATUS_PENDING
or -ENOMEM. Same for slic_config_get

> ?static void slic_init_cleanup(struct adapter *adapter)
> @@ -1265,7 +1265,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address)
> ? ? ? ?mlist = adapter->mcastaddrs;
> ? ? ? ?while (mlist) {
> ? ? ? ? ? ? ? ?if (!compare_ether_addr(mlist->address, address))
> - ? ? ? ? ? ? ? ? ? ? ? return STATUS_SUCCESS;
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> ? ? ? ? ? ? ? ?mlist = mlist->next;
> ? ? ? ?}
>
> @@ -1279,7 +1279,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address)
> ? ? ? ?mcaddr->next = adapter->mcastaddrs;
> ? ? ? ?adapter->mcastaddrs = mcaddr;
>
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?/*
> @@ -1365,7 +1365,7 @@ static void slic_mcast_set_bit(struct adapter *adapter, char *address)
> ?static void slic_mcast_set_list(struct net_device *dev)
> ?{
> ? ? ? ?struct adapter *adapter = netdev_priv(dev);
> - ? ? ? int status = STATUS_SUCCESS;
> + ? ? ? int status = 0;
> ? ? ? ?char *addresses;
> ? ? ? ?struct netdev_hw_addr *ha;
>
> @@ -1374,7 +1374,7 @@ static void slic_mcast_set_list(struct net_device *dev)
> ? ? ? ?netdev_for_each_mc_addr(ha, dev) {
> ? ? ? ? ? ? ? ?addresses = (char *) &ha->addr;
> ? ? ? ? ? ? ? ?status = slic_mcast_add_list(adapter, addresses);
> - ? ? ? ? ? ? ? if (status != STATUS_SUCCESS)
> + ? ? ? ? ? ? ? if (status != 0)
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?slic_mcast_set_bit(adapter, addresses);
> ? ? ? ?}
> @@ -1394,7 +1394,7 @@ static void slic_mcast_set_list(struct net_device *dev)
> ? ? ? ? ? ? ? ?adapter->devflags_prev = dev->flags;
> ? ? ? ? ? ? ? ?slic_config_set(adapter, true);
> ? ? ? ?} else {
> - ? ? ? ? ? ? ? if (status == STATUS_SUCCESS)
> + ? ? ? ? ? ? ? if (status == 0)
> ? ? ? ? ? ? ? ? ? ? ? ?slic_mcast_set_mask(adapter);
> ? ? ? ?}
> ? ? ? ?return;
> @@ -1476,7 +1476,7 @@ static int slic_if_init(struct adapter *adapter)
> ? ? ? ? ? ? ? ? ? ? ? ?adapter->macopts |= MAC_MCAST;
> ? ? ? ?}
> ? ? ? ?status = slic_adapter_allocresources(adapter);
> - ? ? ? if (status != STATUS_SUCCESS) {
> + ? ? ? if (status != 0) {
> ? ? ? ? ? ? ? ?dev_err(&dev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ?"%s: slic_adapter_allocresources FAILED %x\n",
> ? ? ? ? ? ? ? ? ? ? ? ?__func__, status);
> @@ -1553,7 +1553,7 @@ static int slic_if_init(struct adapter *adapter)
> ? ? ? ?slic_link_config(adapter, LINK_AUTOSPEED, LINK_AUTOD);
> ? ? ? ?slic_link_event_handler(adapter);
>
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static void slic_unmap_mmio_space(struct adapter *adapter)
> @@ -1587,7 +1587,7 @@ static int slic_adapter_allocresources(struct adapter *adapter)
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?adapter->intrregistered = 1;
> ? ? ? ?}
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static void slic_config_pci(struct pci_dev *pcidev)
> @@ -1967,7 +1967,7 @@ static int slic_card_download(struct adapter *adapter)
> ? ? ? ? ? and reach mainloop */
> ? ? ? ?mdelay(20);
>
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?MODULE_FIRMWARE("slicoss/oasisdownload.sys");
> @@ -2027,7 +2027,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
> ? ? ? ?/* Download the microcode */
> ? ? ? ?status = slic_card_download(adapter);
>
> - ? ? ? if (status != STATUS_SUCCESS) {
> + ? ? ? if (status != 0) {
> ? ? ? ? ? ? ? ?dev_err(&adapter->pcidev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ?"download failed bus %d slot %d\n",
> ? ? ? ? ? ? ? ? ? ? ? ?adapter->busnumber, adapter->slotnumber);
> @@ -2203,7 +2203,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
> ? ? ? ?card->state = CARD_UP;
> ? ? ? ?card->reset_in_progress = 0;
>
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static u32 slic_card_locate(struct adapter *adapter)
> @@ -2267,7 +2267,7 @@ static u32 slic_card_locate(struct adapter *adapter)
>
> ? ? ? ?ASSERT(card);
> ? ? ? ?if (!card)
> - ? ? ? ? ? ? ? return STATUS_FAILURE;
> + ? ? ? ? ? ? ? return -ENOMEM;

Is -ENOMEM correct for this case?

> ? ? ? ?/* Put the adapter in the card's adapter list */
> ? ? ? ?ASSERT(card->adapter[adapter->port] == NULL);
> ? ? ? ?if (!card->adapter[adapter->port]) {
> @@ -2637,7 +2637,7 @@ static int slic_upr_queue_request(struct adapter *adapter,
> ? ? ? ?} else {
> ? ? ? ? ? ? ? ?adapter->upr_list = upr;
> ? ? ? ?}
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static int slic_upr_request(struct adapter *adapter,
> @@ -2653,7 +2653,7 @@ static int slic_upr_request(struct adapter *adapter,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?upr_request,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?upr_data,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?upr_data_h, upr_buffer, upr_buffer_h);
> - ? ? ? if (status != STATUS_SUCCESS) {
> + ? ? ? if (status != 0) {
> ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&adapter->upr_lock.lock,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?adapter->upr_lock.flags);
> ? ? ? ? ? ? ? ?return status;
> @@ -2661,7 +2661,7 @@ static int slic_upr_request(struct adapter *adapter,
> ? ? ? ?slic_upr_start(adapter);
> ? ? ? ?spin_unlock_irqrestore(&adapter->upr_lock.lock,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?adapter->upr_lock.flags);
> - ? ? ? return STATUS_PENDING;
> + ? ? ? return 0;
> ?}
>
> ?static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
> @@ -3032,7 +3032,7 @@ static int slic_rspqueue_init(struct adapter *adapter)
> ? ? ? ? ? ? ? ? ? ? ? ?dev_err(&adapter->pcidev->dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"pci_alloc_consistent failed\n");
> ? ? ? ? ? ? ? ? ? ? ? ?slic_rspqueue_free(adapter);
> - ? ? ? ? ? ? ? ? ? ? ? return STATUS_FAILURE;
> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> ? ? ? ? ? ? ? ?}
> ?#ifndef CONFIG_X86_64
> ? ? ? ? ? ? ? ?ASSERT(((u32) rspq->vaddr[i] & 0xFFFFF000) ==
> @@ -3056,7 +3056,7 @@ static int slic_rspqueue_init(struct adapter *adapter)
> ? ? ? ?rspq->offset = 0;
> ? ? ? ?rspq->pageindex = 0;
> ? ? ? ?rspq->rspbuf = (struct slic_rspbuf *)rspq->vaddr[0];
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static void slic_rspqueue_free(struct adapter *adapter)
> @@ -3180,13 +3180,13 @@ static int slic_cmdq_init(struct adapter *adapter)
> ?#endif
> ? ? ? ? ? ? ? ?if (!pageaddr) {
> ? ? ? ? ? ? ? ? ? ? ? ?slic_cmdq_free(adapter);
> - ? ? ? ? ? ? ? ? ? ? ? return STATUS_FAILURE;
> + ? ? ? ? ? ? ? ? ? ? ? return -ENOMEM;
> ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?slic_cmdq_addcmdpage(adapter, pageaddr);
> ? ? ? ?}
> ? ? ? ?adapter->slic_handle_ix = 1;
>
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static void slic_cmdq_free(struct adapter *adapter)
> @@ -3407,9 +3407,9 @@ static int slic_rcvqueue_init(struct adapter *adapter)
> ? ? ? ?}
> ? ? ? ?if (rcvq->count < SLIC_RCVQ_MINENTRIES) {
> ? ? ? ? ? ? ? ?slic_rcvqueue_free(adapter);
> - ? ? ? ? ? ? ? return STATUS_FAILURE;
> + ? ? ? ? ? ? ? return -ENOMEM;
> ? ? ? ?}
> - ? ? ? return STATUS_SUCCESS;
> + ? ? ? return 0;
> ?}
>
> ?static void slic_rcvqueue_free(struct adapter *adapter)
> --
> 1.7.0.4
>
>



--
Regards,
Denis

2010-06-28 11:14:15

by Kulikov Vasiliy

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.

> > @@ -1206,7 +1206,7 @@ static void slic_link_event_handler(struct adapter *adapter)
> > ?#else
> > ? ? ? ?Stop compilation;
> > ?#endif
> > - ? ? ? ASSERT((status == STATUS_SUCCESS) || (status == STATUS_PENDING));
> > + ? ? ? ASSERT(status == 0);
> > ?}
> >
>
> Now that looks useless since slic_upr_request can return STATUS_PENDING
> or -ENOMEM. Same for slic_config_get
Yes, it seems that slic_link_event_handler & slic_config_get have to return error codes.

> > @@ -2267,7 +2267,7 @@ static u32 slic_card_locate(struct adapter *adapter)
> >
> > ? ? ? ?ASSERT(card);
> > ? ? ? ?if (!card)
> > - ? ? ? ? ? ? ? return STATUS_FAILURE;
> > + ? ? ? ? ? ? ? return -ENOMEM;
>
> Is -ENOMEM correct for this case?
>
Right, maybe ENXIO?

2010-06-28 14:12:52

by Denis Kirjanov

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.

On 06/28/2010 03:14 PM, Kulikov Vasiliy wrote:
>>> @@ -1206,7 +1206,7 @@ static void slic_link_event_handler(struct adapter *adapter)
>>> #else
>>> Stop compilation;
>>> #endif
>>> - ASSERT((status == STATUS_SUCCESS) || (status == STATUS_PENDING));
>>> + ASSERT(status == 0);
>>> }
>>>
>>
>> Now that looks useless since slic_upr_request can return STATUS_PENDING
>> or -ENOMEM. Same for slic_config_get
> Yes, it seems that slic_link_event_handler& slic_config_get have to return error codes.
>
>>> @@ -2267,7 +2267,7 @@ static u32 slic_card_locate(struct adapter *adapter)
>>>
>>> ASSERT(card);
>>> if (!card)
>>> - return STATUS_FAILURE;
>>> + return -ENOMEM;
>>
>> Is -ENOMEM correct for this case?
>>
> Right, maybe ENXIO?
>
Yes, I'm fine with this.

2010-06-30 13:02:46

by Kulikov Vasiliy

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.

On Mon, Jun 28, 2010 at 14:12 +0400, Denis Kirjanov wrote:
> > diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> > index bebf0fd..102d3ea 100644
> > --- a/drivers/staging/slicoss/slicoss.c
> > +++ b/drivers/staging/slicoss/slicoss.c
> > @@ -452,7 +452,7 @@ static int __devinit slic_entry_probe(struct pci_dev *pcidev,
> >
> > ? ? ? ?status = slic_card_init(card, adapter);
> >
> > - ? ? ? if (status != STATUS_SUCCESS) {
> > + ? ? ? if (status != 0) {
> > ? ? ? ? ? ? ? ?card->state = CARD_FAIL;
> > ? ? ? ? ? ? ? ?adapter->state = ADAPT_FAIL;
> > ? ? ? ? ? ? ? ?adapter->linkstate = LINK_DOWN;
>
> Can we really continue here?
>

It seems that we have to goto err_out_unmap, yes?

> > @@ -1206,7 +1206,7 @@ static void slic_link_event_handler(struct adapter *adapter)
> > ?#else
> > ? ? ? ?Stop compilation;
> > ?#endif
> > - ? ? ? ASSERT((status == STATUS_SUCCESS) || (status == STATUS_PENDING));
> > + ? ? ? ASSERT(status == 0);
> > ?}
> >
>
> Now that looks useless since slic_upr_request can return STATUS_PENDING
> or -ENOMEM. Same for slic_config_get


Anyway, this code is full of ASSERT()'s, grep see 71 calls to it.
It needs more considered patch than these cleanup patches.

2010-06-30 13:02:54

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.

Change defined STATUS_XXX return codes to standard -EYYY.

Signed-off-by: Kulikov Vasiliy <[email protected]>
---
drivers/staging/slicoss/slicoss.c | 52 ++++++++++++++++++------------------
1 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index bebf0fd..16cd2cb 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -452,7 +452,7 @@ static int __devinit slic_entry_probe(struct pci_dev *pcidev,

status = slic_card_init(card, adapter);

- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
card->state = CARD_FAIL;
adapter->state = ADAPT_FAIL;
adapter->linkstate = LINK_DOWN;
@@ -513,7 +513,7 @@ static int slic_entry_open(struct net_device *dev)
}
status = slic_if_init(adapter);

- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
if (adapter->activated) {
card->adapters_activated--;
slic_global.num_slic_ports_active--;
@@ -535,7 +535,7 @@ static int slic_entry_open(struct net_device *dev)
locked = 0;
}

- return STATUS_SUCCESS;
+ return 0;
}

static void __devexit slic_entry_remove(struct pci_dev *pcidev)
@@ -628,7 +628,7 @@ static int slic_entry_halt(struct net_device *dev)

spin_unlock_irqrestore(&slic_global.driver_lock.lock,
slic_global.driver_lock.flags);
- return STATUS_SUCCESS;
+ return 0;
}

static int slic_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
@@ -1206,7 +1206,7 @@ static void slic_link_event_handler(struct adapter *adapter)
#else
Stop compilation;
#endif
- ASSERT((status == STATUS_SUCCESS) || (status == STATUS_PENDING));
+ ASSERT(status == 0);
}

static void slic_init_cleanup(struct adapter *adapter)
@@ -1265,7 +1265,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address)
mlist = adapter->mcastaddrs;
while (mlist) {
if (!compare_ether_addr(mlist->address, address))
- return STATUS_SUCCESS;
+ return 0;
mlist = mlist->next;
}

@@ -1279,7 +1279,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address)
mcaddr->next = adapter->mcastaddrs;
adapter->mcastaddrs = mcaddr;

- return STATUS_SUCCESS;
+ return 0;
}

/*
@@ -1365,7 +1365,7 @@ static void slic_mcast_set_bit(struct adapter *adapter, char *address)
static void slic_mcast_set_list(struct net_device *dev)
{
struct adapter *adapter = netdev_priv(dev);
- int status = STATUS_SUCCESS;
+ int status = 0;
char *addresses;
struct netdev_hw_addr *ha;

@@ -1374,7 +1374,7 @@ static void slic_mcast_set_list(struct net_device *dev)
netdev_for_each_mc_addr(ha, dev) {
addresses = (char *) &ha->addr;
status = slic_mcast_add_list(adapter, addresses);
- if (status != STATUS_SUCCESS)
+ if (status != 0)
break;
slic_mcast_set_bit(adapter, addresses);
}
@@ -1394,7 +1394,7 @@ static void slic_mcast_set_list(struct net_device *dev)
adapter->devflags_prev = dev->flags;
slic_config_set(adapter, true);
} else {
- if (status == STATUS_SUCCESS)
+ if (status == 0)
slic_mcast_set_mask(adapter);
}
return;
@@ -1476,7 +1476,7 @@ static int slic_if_init(struct adapter *adapter)
adapter->macopts |= MAC_MCAST;
}
status = slic_adapter_allocresources(adapter);
- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
dev_err(&dev->dev,
"%s: slic_adapter_allocresources FAILED %x\n",
__func__, status);
@@ -1553,7 +1553,7 @@ static int slic_if_init(struct adapter *adapter)
slic_link_config(adapter, LINK_AUTOSPEED, LINK_AUTOD);
slic_link_event_handler(adapter);

- return STATUS_SUCCESS;
+ return 0;
}

static void slic_unmap_mmio_space(struct adapter *adapter)
@@ -1587,7 +1587,7 @@ static int slic_adapter_allocresources(struct adapter *adapter)
}
adapter->intrregistered = 1;
}
- return STATUS_SUCCESS;
+ return 0;
}

static void slic_config_pci(struct pci_dev *pcidev)
@@ -1967,7 +1967,7 @@ static int slic_card_download(struct adapter *adapter)
and reach mainloop */
mdelay(20);

- return STATUS_SUCCESS;
+ return 0;
}

MODULE_FIRMWARE("slicoss/oasisdownload.sys");
@@ -2027,7 +2027,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
/* Download the microcode */
status = slic_card_download(adapter);

- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
dev_err(&adapter->pcidev->dev,
"download failed bus %d slot %d\n",
adapter->busnumber, adapter->slotnumber);
@@ -2203,7 +2203,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
card->state = CARD_UP;
card->reset_in_progress = 0;

- return STATUS_SUCCESS;
+ return 0;
}

static u32 slic_card_locate(struct adapter *adapter)
@@ -2267,7 +2267,7 @@ static u32 slic_card_locate(struct adapter *adapter)

ASSERT(card);
if (!card)
- return STATUS_FAILURE;
+ return -ENXIO;
/* Put the adapter in the card's adapter list */
ASSERT(card->adapter[adapter->port] == NULL);
if (!card->adapter[adapter->port]) {
@@ -2637,7 +2637,7 @@ static int slic_upr_queue_request(struct adapter *adapter,
} else {
adapter->upr_list = upr;
}
- return STATUS_SUCCESS;
+ return 0;
}

static int slic_upr_request(struct adapter *adapter,
@@ -2653,7 +2653,7 @@ static int slic_upr_request(struct adapter *adapter,
upr_request,
upr_data,
upr_data_h, upr_buffer, upr_buffer_h);
- if (status != STATUS_SUCCESS) {
+ if (status != 0) {
spin_unlock_irqrestore(&adapter->upr_lock.lock,
adapter->upr_lock.flags);
return status;
@@ -2661,7 +2661,7 @@ static int slic_upr_request(struct adapter *adapter,
slic_upr_start(adapter);
spin_unlock_irqrestore(&adapter->upr_lock.lock,
adapter->upr_lock.flags);
- return STATUS_PENDING;
+ return 0;
}

static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
@@ -3032,7 +3032,7 @@ static int slic_rspqueue_init(struct adapter *adapter)
dev_err(&adapter->pcidev->dev,
"pci_alloc_consistent failed\n");
slic_rspqueue_free(adapter);
- return STATUS_FAILURE;
+ return -ENOMEM;
}
#ifndef CONFIG_X86_64
ASSERT(((u32) rspq->vaddr[i] & 0xFFFFF000) ==
@@ -3056,7 +3056,7 @@ static int slic_rspqueue_init(struct adapter *adapter)
rspq->offset = 0;
rspq->pageindex = 0;
rspq->rspbuf = (struct slic_rspbuf *)rspq->vaddr[0];
- return STATUS_SUCCESS;
+ return 0;
}

static void slic_rspqueue_free(struct adapter *adapter)
@@ -3180,13 +3180,13 @@ static int slic_cmdq_init(struct adapter *adapter)
#endif
if (!pageaddr) {
slic_cmdq_free(adapter);
- return STATUS_FAILURE;
+ return -ENOMEM;
}
slic_cmdq_addcmdpage(adapter, pageaddr);
}
adapter->slic_handle_ix = 1;

- return STATUS_SUCCESS;
+ return 0;
}

static void slic_cmdq_free(struct adapter *adapter)
@@ -3407,9 +3407,9 @@ static int slic_rcvqueue_init(struct adapter *adapter)
}
if (rcvq->count < SLIC_RCVQ_MINENTRIES) {
slic_rcvqueue_free(adapter);
- return STATUS_FAILURE;
+ return -ENOMEM;
}
- return STATUS_SUCCESS;
+ return 0;
}

static void slic_rcvqueue_free(struct adapter *adapter)
--
1.7.0.4

2010-07-08 20:16:44

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch

On Sun, Jun 27, 2010 at 05:20:45PM +0400, Kulikov Vasiliy wrote:
> Move NULL pointer assertion to else branch because it can
> become NULL only here.
>

This patch doesn't apply on the linux-next tree :(

thanks,

greg k-h