linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.
@ 2010-06-27 13:20 Kulikov Vasiliy
  2010-06-27 13:20 ` [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch Kulikov Vasiliy
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-27 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lior Dotan, charrer, Denis Kirjanov, David S. Miller, Jiri Pirko,
	devel, linux-kernel

Change defined STATUS_XXX return codes to standard -EYYY.

Signed-off-by: Kulikov Vasiliy <segooon@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;
@@ -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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch
  2010-06-27 13:20 [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Kulikov Vasiliy
@ 2010-06-27 13:20 ` Kulikov Vasiliy
  2010-07-08 20:11   ` Greg KH
  2010-06-27 13:20 ` [PATCH 4/5] staging: slicoss: error handling with goto Kulikov Vasiliy
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-27 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lior Dotan, charrer, Denis Kirjanov, David S. Miller, Jiri Pirko,
	devel, linux-kernel

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

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] staging: slicoss: error handling with goto
  2010-06-27 13:20 [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Kulikov Vasiliy
  2010-06-27 13:20 ` [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch Kulikov Vasiliy
@ 2010-06-27 13:20 ` Kulikov Vasiliy
  2010-06-27 13:20 ` [PATCH 5/5] " Kulikov Vasiliy
  2010-06-28 10:12 ` [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Denis Kirjanov
  3 siblings, 0 replies; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-27 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lior Dotan, charrer, Denis Kirjanov, David S. Miller, Jiri Pirko,
	devel, linux-kernel

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

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] staging: slicoss: error handling with goto
  2010-06-27 13:20 [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Kulikov Vasiliy
  2010-06-27 13:20 ` [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch Kulikov Vasiliy
  2010-06-27 13:20 ` [PATCH 4/5] staging: slicoss: error handling with goto Kulikov Vasiliy
@ 2010-06-27 13:20 ` Kulikov Vasiliy
  2010-06-28 10:12 ` [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Denis Kirjanov
  3 siblings, 0 replies; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-27 13:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Lior Dotan, charrer, Denis Kirjanov, David S. Miller, Jiri Pirko,
	devel, linux-kernel

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

Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.
  2010-06-27 13:20 [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Kulikov Vasiliy
                   ` (2 preceding siblings ...)
  2010-06-27 13:20 ` [PATCH 5/5] " Kulikov Vasiliy
@ 2010-06-28 10:12 ` Denis Kirjanov
  2010-06-28 11:14   ` Kulikov Vasiliy
  2010-06-30 13:02   ` Kulikov Vasiliy
  3 siblings, 2 replies; 10+ messages in thread
From: Denis Kirjanov @ 2010-06-28 10:12 UTC (permalink / raw)
  To: Kulikov Vasiliy
  Cc: Greg Kroah-Hartman, Lior Dotan, charrer, David S. Miller,
	Jiri Pirko, devel, linux-kernel

On Sun, Jun 27, 2010 at 5:20 PM, Kulikov Vasiliy <segooon@gmail.com> wrote:
> Change defined STATUS_XXX return codes to standard -EYYY.
>
> Signed-off-by: Kulikov Vasiliy <segooon@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.
  2010-06-28 10:12 ` [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Denis Kirjanov
@ 2010-06-28 11:14   ` Kulikov Vasiliy
  2010-06-28 14:12     ` Denis Kirjanov
  2010-06-30 13:02   ` Kulikov Vasiliy
  1 sibling, 1 reply; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-28 11:14 UTC (permalink / raw)
  To: dkirjanov
  Cc: Greg Kroah-Hartman, Lior Dotan, charrer, David S. Miller,
	Jiri Pirko, devel, linux-kernel

> > @@ -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?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.
  2010-06-28 11:14   ` Kulikov Vasiliy
@ 2010-06-28 14:12     ` Denis Kirjanov
  2010-06-30 13:02       ` Kulikov Vasiliy
  0 siblings, 1 reply; 10+ messages in thread
From: Denis Kirjanov @ 2010-06-28 14:12 UTC (permalink / raw)
  To: Kulikov Vasiliy
  Cc: Greg Kroah-Hartman, Lior Dotan, charrer, David S. Miller,
	Jiri Pirko, devel, linux-kernel

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.
  2010-06-28 10:12 ` [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Denis Kirjanov
  2010-06-28 11:14   ` Kulikov Vasiliy
@ 2010-06-30 13:02   ` Kulikov Vasiliy
  1 sibling, 0 replies; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-30 13:02 UTC (permalink / raw)
  To: dkirjanov
  Cc: Greg Kroah-Hartman, Lior Dotan, charrer, David S. Miller,
	Jiri Pirko, devel, linux-kernel

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] staging: slicoss: Change return codes to -EYYY.
  2010-06-28 14:12     ` Denis Kirjanov
@ 2010-06-30 13:02       ` Kulikov Vasiliy
  0 siblings, 0 replies; 10+ messages in thread
From: Kulikov Vasiliy @ 2010-06-30 13:02 UTC (permalink / raw)
  To: Kernel Janitors
  Cc: Greg Kroah-Hartman, Denis Kirjanov, David S. Miller, Jiri Pirko,
	Ben Hutchings, devel, linux-kernel

Change defined STATUS_XXX return codes to standard -EYYY.

Signed-off-by: Kulikov Vasiliy <segooon@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..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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch
  2010-06-27 13:20 ` [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch Kulikov Vasiliy
@ 2010-07-08 20:11   ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2010-07-08 20:11 UTC (permalink / raw)
  To: Kulikov Vasiliy
  Cc: Greg Kroah-Hartman, Lior Dotan, charrer, Denis Kirjanov,
	David S. Miller, Jiri Pirko, devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-07-08 20:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-27 13:20 [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Kulikov Vasiliy
2010-06-27 13:20 ` [PATCH 3/5] staging: slicoss: Move NULL pointer assertion to else branch Kulikov Vasiliy
2010-07-08 20:11   ` Greg KH
2010-06-27 13:20 ` [PATCH 4/5] staging: slicoss: error handling with goto Kulikov Vasiliy
2010-06-27 13:20 ` [PATCH 5/5] " Kulikov Vasiliy
2010-06-28 10:12 ` [PATCH 1/5] staging: slicoss: Change return codes to -EYYY Denis Kirjanov
2010-06-28 11:14   ` Kulikov Vasiliy
2010-06-28 14:12     ` Denis Kirjanov
2010-06-30 13:02       ` Kulikov Vasiliy
2010-06-30 13:02   ` Kulikov Vasiliy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).