From: Denis Kirjanov on
On Sun, Jun 27, 2010 at 5:20 PM, Kulikov Vasiliy <segooon(a)gmail.com> wrote:
> Change defined STATUS_XXX return codes to standard -EYYY.
>
> Signed-off-by: Kulikov Vasiliy <segooon(a)gmail.com>
> ---
> �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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kulikov Vasiliy on
> > @@ -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?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Denis Kirjanov on
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kulikov Vasiliy on
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/