Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1546989 > unrolled thread

[PATCH] drivers: net: ethernet: 3com: fix return value

Started byThomas Preisner <thomas.preisner+linux@fau.de>
First post2016-12-24 00:10 +0100
Last post2016-12-24 13:10 +0100
Articles 18 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-24 00:10 +0100
    Re: [PATCH] drivers: net: ethernet: 3com: fix return value David Dillow <dave@thedillows.org> - 2016-12-24 02:20 +0100
      [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-24 13:10 +0100
        Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-12-24 20:10 +0100
          [PATCH v3 1/2] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-25 01:40 +0100
            Re: [PATCH v3 1/2] drivers: net: ethernet: 3com: fix return value David Miller <davem@davemloft.net> - 2016-12-25 02:00 +0100
          [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-25 01:40 +0100
            Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-12-25 11:10 +0100
            Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value David Dillow <dave@thedillows.org> - 2016-12-27 22:20 +0100
              [PATCH v4 1/2] net: 3com: typhoon: typhoon_init_one: fix incorrect return values Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-30 03:40 +0100
                Re: [PATCH v4 1/2] net: 3com: typhoon: typhoon_init_one: fix  incorrect return values David Miller <davem@davemloft.net> - 2016-12-30 21:30 +0100
              [PATCH v4 2/2] net: 3com: typhoon: typhoon_init_one: make return values more specific Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-30 03:40 +0100
                Re: [PATCH v4 2/2] net: 3com: typhoon: typhoon_init_one: make  return values more specific David Miller <davem@davemloft.net> - 2016-12-30 21:30 +0100
              Re: Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-30 03:40 +0100
          Re: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-25 01:40 +0100
            Re: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return  value David Dillow <dave@thedillows.org> - 2016-12-27 22:20 +0100
      Re: [PATCH] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-24 13:10 +0100
        [PATCH v2 2/2] drivers: net: ethernet: 3com: fix return value Thomas Preisner <thomas.preisner+linux@fau.de> - 2016-12-24 13:10 +0100

#1546989 — [PATCH] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-24 00:10 +0100
Subject[PATCH] drivers: net: ethernet: 3com: fix return value
Message-ID<sRH8l-4S6-11@gated-at.bofh.it>
In a few cases the err-variable is not set to a negative error code if a
function call fails and thus 0 is returned instead.
It may be better to set err to the proper negative error code before
returning.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841

Reported-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index a0cacbe..9a3ab58 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2404,6 +2404,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	if(!is_valid_ether_addr(dev->dev_addr)) {
 		err_msg = "Could not obtain valid ethernet address, aborting";
+		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2413,6 +2414,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
 	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
 		err_msg = "Could not get Sleep Image version";
+		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2455,6 +2457,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	if(register_netdev(dev) < 0) {
 		err_msg = "unable to register netdev";
+		err = -EIO;
 		goto error_out_reset;
 	}
 
-- 
2.7.4

[toc] | [next] | [standalone]

#1546996

FromDavid Dillow <dave@thedillows.org>
Date2016-12-24 02:20 +0100
Message-ID<sRJa9-6Gi-3@gated-at.bofh.it>
In reply to#1546989
On Sat, 2016-12-24 at 00:00 +0100, Thomas Preisner wrote:
> diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
> index a0cacbe..9a3ab58 100644
> --- a/drivers/net/ethernet/3com/typhoon.c
> +++ b/drivers/net/ethernet/3com/typhoon.c
> @@ -2404,6 +2404,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	if(!is_valid_ether_addr(dev->dev_addr)) {
>  		err_msg = "Could not obtain valid ethernet address, aborting";
> +		err = -EIO;
>  		goto error_out_reset;

The change above is fine, but the other two should use the return value
from the failing function call.


> @@ -2413,6 +2414,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
>  	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
>  		err_msg = "Could not get Sleep Image version";
> +		err = -EIO;
>  		goto error_out_reset;
>  	}
>  
> @@ -2455,6 +2457,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	if(register_netdev(dev) < 0) {
>  		err_msg = "unable to register netdev";
> +		err = -EIO;
>  		goto error_out_reset;
>  	}
>

[toc] | [prev] | [next] | [standalone]

#1547050 — [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-24 13:10 +0100
Subject[PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sRTjb-6KI-1@gated-at.bofh.it>
In reply to#1546996
In a few cases the err-variable is not set to a negative error code if a
function call fails and thus 0 is returned instead.
It may be better to set err to the appropriate negative error code
before returning.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841

Reported-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index a0cacbe..c88b88a 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2404,6 +2404,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	if(!is_valid_ether_addr(dev->dev_addr)) {
 		err_msg = "Could not obtain valid ethernet address, aborting";
+		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * later when we print out the version reported.
 	 */
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
-	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
+	err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
+	if(err < 0) {
 		err_msg = "Could not get Sleep Image version";
 		goto error_out_reset;
 	}
@@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->features = dev->hw_features |
 		NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
 
-	if(register_netdev(dev) < 0) {
+	err = register_netdev(dev);
+	if(err < 0) {
 		err_msg = "unable to register netdev";
 		goto error_out_reset;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1547105 — Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-12-24 20:10 +0100
SubjectRe: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sRZRE-2r4-11@gated-at.bofh.it>
In reply to#1547050
Hello!

On 12/24/2016 03:02 PM, Thomas Preisner wrote:

> In a few cases the err-variable is not set to a negative error code if a
> function call fails and thus 0 is returned instead.
> It may be better to set err to the appropriate negative error code
> before returning.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841
>
> Reported-by: Pan Bian <bianpan2016@163.com>
> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
> ---
>  drivers/net/ethernet/3com/typhoon.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
> index a0cacbe..c88b88a 100644
> --- a/drivers/net/ethernet/3com/typhoon.c
> +++ b/drivers/net/ethernet/3com/typhoon.c
[...]
> @@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	 * later when we print out the version reported.
>  	 */
>  	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
> -	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
> +	err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
> +	if(err < 0) {

    Need a space between *if* and (. Run your patches thru 
scripts/checkpatch.pl before posting, please.

>  		err_msg = "Could not get Sleep Image version";
>  		goto error_out_reset;
>  	}
> @@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	dev->features = dev->hw_features |
>  		NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
>
> -	if(register_netdev(dev) < 0) {
> +	err = register_netdev(dev);
> +	if(err < 0) {

    Same here.

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1547111 — [PATCH v3 1/2] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-25 01:40 +0100
Subject[PATCH v3 1/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sS50Z-5Dz-13@gated-at.bofh.it>
In reply to#1547105
In a few cases the err-variable is not set to a negative error code if a
function call fails and thus 0 is returned instead.
It may be better to set err to the appropriate negative error code
before returning.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841

Reported-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index a0cacbe..c88b88a 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2404,6 +2404,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
-	if(!is_valid_ether_addr(dev->dev_addr)) {
+	if (!is_valid_ether_addr(dev->dev_addr)) {
 		err_msg = "Could not obtain valid ethernet address, aborting";
+		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * later when we print out the version reported.
 	 */
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
-	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
+	err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
+	if (err < 0) {
 		err_msg = "Could not get Sleep Image version";
 		goto error_out_reset;
 	}
@@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->features = dev->hw_features |
 		NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
 
-	if(register_netdev(dev) < 0) {
+	err = register_netdev(dev);
+	if (err < 0) {
 		err_msg = "unable to register netdev";
 		goto error_out_reset;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1547114 — Re: [PATCH v3 1/2] drivers: net: ethernet: 3com: fix return value

FromDavid Miller <davem@davemloft.net>
Date2016-12-25 02:00 +0100
SubjectRe: [PATCH v3 1/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sS5kl-5K9-5@gated-at.bofh.it>
In reply to#1547111
It is never, ever, appropriate to use the same exact Subject: line
text for two different changes.

Someone looking at "git shortlog" has no way to know what is different
between the two changes.

You must put care and time into constructing Subject: lines because
this text is critical for data mining and analysis done by both humans
and machines.

[toc] | [prev] | [next] | [standalone]

#1547112 — [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-25 01:40 +0100
Subject[PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sS50Z-5Dz-9@gated-at.bofh.it>
In reply to#1547105
In some cases the return value of a failing function is not being used
and the function typhoon_init_one() returns another negative error
code instead.

Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index c88b88a..8821a24 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2370,9 +2370,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * 4) Get the hardware address.
 	 * 5) Put the card to sleep.
 	 */
-	if (typhoon_reset(ioaddr, WaitSleep) < 0) {
+	err = typhoon_reset(ioaddr, WaitSleep);
+	if (err < 0) {
 		err_msg = "could not reset 3XP";
-		err = -EIO;
 		goto error_out_dma;
 	}
 
@@ -2386,16 +2386,16 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	typhoon_init_interface(tp);
 	typhoon_init_rings(tp);
 
-	if(typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST) < 0) {
+	err = typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST);
+	if (err < 0) {
 		err_msg = "cannot boot 3XP sleep image";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_MAC_ADDRESS);
-	if(typhoon_issue_command(tp, 1, &xp_cmd, 1, xp_resp) < 0) {
+	err = typhoon_issue_command(tp, 1, &xp_cmd, 1, xp_resp);
+	if (err < 0) {
 		err_msg = "cannot read MAC address";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2430,9 +2430,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if(xp_resp[0].numDesc != 0)
 		tp->capabilities |= TYPHOON_WAKEUP_NEEDS_RESET;
 
-	if(typhoon_sleep(tp, PCI_D3hot, 0) < 0) {
+	err = typhoon_sleep(tp, PCI_D3hot, 0);
+	if (err < 0) {
 		err_msg = "cannot put adapter to sleep";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1547148 — Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-12-25 11:10 +0100
SubjectRe: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sSdUB-3vV-25@gated-at.bofh.it>
In reply to#1547112
Hello!

On 12/25/2016 3:30 AM, Thomas Preisner wrote:

> In some cases the return value of a failing function is not being used
> and the function typhoon_init_one() returns another negative error
> code instead.
>
> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
> ---
>  drivers/net/ethernet/3com/typhoon.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

    In addition to what DaveM said, your choise of the subject prefixes is too 
wide -- it would seem that you're fixing all 3com drivers, while you're only 
fixing typhoon. That "typhoon:" alone would have been an appropriate prefix.

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1547753 — Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

FromDavid Dillow <dave@thedillows.org>
Date2016-12-27 22:20 +0100
SubjectRe: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sT7k5-7ne-11@gated-at.bofh.it>
In reply to#1547112
On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote:
> In some cases the return value of a failing function is not being used
> and the function typhoon_init_one() returns another negative error
> code instead.

I'm not sure these changes are especially valuable, since we'll need to
look at the dmesg log anyways to figure out what went wrong, but again I
don't feel strongly.

Fix up the subject issues and I'm happy to ack them.

[toc] | [prev] | [next] | [standalone]

#1548432 — [PATCH v4 1/2] net: 3com: typhoon: typhoon_init_one: fix incorrect return values

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-30 03:40 +0100
Subject[PATCH v4 1/2] net: 3com: typhoon: typhoon_init_one: fix incorrect return values
Message-ID<sTVgR-6Gr-11@gated-at.bofh.it>
In reply to#1547753
In a few cases the err-variable is not set to a negative error code if a
function call in typhoon_init_one() fails and thus 0 is returned
instead.
It may be better to set err to the appropriate negative error
code before returning.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841

Reported-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 9fe3990..25f2e92 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2402,8 +2402,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	*(__be16 *)&dev->dev_addr[0] = htons(le16_to_cpu(xp_resp[0].parm1));
 	*(__be32 *)&dev->dev_addr[2] = htonl(le32_to_cpu(xp_resp[0].parm2));
 
-	if(!is_valid_ether_addr(dev->dev_addr)) {
+	if (!is_valid_ether_addr(dev->dev_addr)) {
 		err_msg = "Could not obtain valid ethernet address, aborting";
+		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * later when we print out the version reported.
 	 */
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
-	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
+	err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
+	if (err < 0) {
 		err_msg = "Could not get Sleep Image version";
 		goto error_out_reset;
 	}
@@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->features = dev->hw_features |
 		NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
 
-	if(register_netdev(dev) < 0) {
+	err = register_netdev(dev);
+	if (err < 0) {
 		err_msg = "unable to register netdev";
 		goto error_out_reset;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1548732 — Re: [PATCH v4 1/2] net: 3com: typhoon: typhoon_init_one: fix incorrect return values

FromDavid Miller <davem@davemloft.net>
Date2016-12-30 21:30 +0100
SubjectRe: [PATCH v4 1/2] net: 3com: typhoon: typhoon_init_one: fix incorrect return values
Message-ID<sUbYl-GW-5@gated-at.bofh.it>
In reply to#1548432
From: Thomas Preisner <thomas.preisner+linux@fau.de>
Date: Fri, 30 Dec 2016 03:37:53 +0100

> In a few cases the err-variable is not set to a negative error code if a
> function call in typhoon_init_one() fails and thus 0 is returned
> instead.
> It may be better to set err to the appropriate negative error
> code before returning.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841
> 
> Reported-by: Pan Bian <bianpan2016@163.com>
> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>

Applied to net-next.

[toc] | [prev] | [next] | [standalone]

#1548434 — [PATCH v4 2/2] net: 3com: typhoon: typhoon_init_one: make return values more specific

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-30 03:40 +0100
Subject[PATCH v4 2/2] net: 3com: typhoon: typhoon_init_one: make return values more specific
Message-ID<sTVgR-6Gr-13@gated-at.bofh.it>
In reply to#1547753
In some cases the return value of a failing function is not being used
and the function typhoon_init_one() returns another negative error code
instead.

Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 25f2e92..1986ad1 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2370,9 +2370,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * 4) Get the hardware address.
 	 * 5) Put the card to sleep.
 	 */
-	if (typhoon_reset(ioaddr, WaitSleep) < 0) {
+	err = typhoon_reset(ioaddr, WaitSleep);
+	if (err < 0) {
 		err_msg = "could not reset 3XP";
-		err = -EIO;
 		goto error_out_dma;
 	}
 
@@ -2386,16 +2386,16 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	typhoon_init_interface(tp);
 	typhoon_init_rings(tp);
 
-	if(typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST) < 0) {
+	err = typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST);
+	if (err < 0) {
 		err_msg = "cannot boot 3XP sleep image";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_MAC_ADDRESS);
-	if(typhoon_issue_command(tp, 1, &xp_cmd, 1, xp_resp) < 0) {
+	err = typhoon_issue_command(tp, 1, &xp_cmd, 1, xp_resp);
+	if (err < 0) {
 		err_msg = "cannot read MAC address";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2430,9 +2430,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if(xp_resp[0].numDesc != 0)
 		tp->capabilities |= TYPHOON_WAKEUP_NEEDS_RESET;
 
-	if(typhoon_sleep(tp, PCI_D3hot, 0) < 0) {
+	err = typhoon_sleep(tp, PCI_D3hot, 0);
+	if (err < 0) {
 		err_msg = "cannot put adapter to sleep";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1548735 — Re: [PATCH v4 2/2] net: 3com: typhoon: typhoon_init_one: make return values more specific

FromDavid Miller <davem@davemloft.net>
Date2016-12-30 21:30 +0100
SubjectRe: [PATCH v4 2/2] net: 3com: typhoon: typhoon_init_one: make return values more specific
Message-ID<sUbYl-GW-13@gated-at.bofh.it>
In reply to#1548434
From: Thomas Preisner <thomas.preisner+linux@fau.de>
Date: Fri, 30 Dec 2016 03:37:54 +0100

> In some cases the return value of a failing function is not being used
> and the function typhoon_init_one() returns another negative error code
> instead.
> 
> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>

Applied to net-next.

[toc] | [prev] | [next] | [standalone]

#1548435 — Re: Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-30 03:40 +0100
SubjectRe: Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sTVgR-6Gr-3@gated-at.bofh.it>
In reply to#1547753
On Tue, 2016-12-27 at 22:17:35 +0100, David Dillow wrote:
>On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote:
>> In some cases the return value of a failing function is not being used
>> and the function typhoon_init_one() returns another negative error
>> code instead.
>
>I'm not sure these changes are especially valuable, since we'll need to
>look at the dmesg log anyways to figure out what went wrong, but again I
>don't feel strongly.
>
>Fix up the subject issues and I'm happy to ack them.

As requested, here are the patchsets with the fixed subjects.
The subjects aswell as the subject prefixes are more precise now.
Hopefully that's ok.

Patch 1:
Makes the function typhoon_init_one() return a negative error code instead of 0.

Patch 2 [Optional]:
Makes the function typhoon_init_one() return the return value of the
corresponding failing function calls instead of a "fixed" negative error code.

With Regards,
Milan and Thomas

[toc] | [prev] | [next] | [standalone]

#1547113 — Re: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-25 01:40 +0100
SubjectRe: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sS50Z-5Dz-11@gated-at.bofh.it>
In reply to#1547105
Hello.

On Sat, 2016-12-24 at 20:06 +0100, Sergei Shtylyov wrote:
>Hello!
>
>On 12/24/2016 03:02 PM, Thomas Preisner wrote:
>
>> In a few cases the err-variable is not set to a negative error code if a
>> function call fails and thus 0 is returned instead.
>> It may be better to set err to the appropriate negative error code
>> before returning.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841
>>
>> Reported-by: Pan Bian <bianpan2016@163.com>
>> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
>> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
>> ---
>>  drivers/net/ethernet/3com/typhoon.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
>> index a0cacbe..c88b88a 100644
>> --- a/drivers/net/ethernet/3com/typhoon.c
>> +++ b/drivers/net/ethernet/3com/typhoon.c
>[...]
>> @@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	 * later when we print out the version reported.
>>  	 */
>>  	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
>> -	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
>> +	err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
>> +	if(err < 0) {
>
>    Need a space between *if* and (. Run your patches thru 
>scripts/checkpatch.pl before posting, please.

Those spaces were actually left out purposely: The file in question (typhoon.c)
is missing those spaces between the statements (if, for, while) and the
following opening bracket pretty much always (except 2-3 times) and we figured
that it might be better to keep the coding style consistent since this might
aswell have been intended by the original author.

>
>>  		err_msg = "Could not get Sleep Image version";
>>  		goto error_out_reset;
>>  	}
>> @@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	dev->features = dev->hw_features |
>>  		NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
>>
>> -	if(register_netdev(dev) < 0) {
>> +	err = register_netdev(dev);
>> +	if(err < 0) {
>
>    Same here.
>
>[...]
>
>MBR, Sergei

But of course we can provide you with a patchset including those spaces.

With Regards,
Milan and Thomas

[toc] | [prev] | [next] | [standalone]

#1547754 — Re: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value

FromDavid Dillow <dave@thedillows.org>
Date2016-12-27 22:20 +0100
SubjectRe: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sT7k5-7ne-13@gated-at.bofh.it>
In reply to#1547113
On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote:
> Those spaces were actually left out purposely: The file in question (typhoon.c)
> is missing those spaces between the statements (if, for, while) and the
> following opening bracket pretty much always (except 2-3 times) and we figured
> that it might be better to keep the coding style consistent since this might
> aswell have been intended by the original author.

I'm not sure if we had the rule back then, or if I just missed it.
Either way, we should follow the rules for new code if we can.

I'm not sure it's worth fixing all of the instances -- usually
formatting-only changes are not worth the churn -- but I don't have a
strong opinion on the matter.

[toc] | [prev] | [next] | [standalone]

#1547051

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-24 13:10 +0100
Message-ID<sRTjb-6KI-3@gated-at.bofh.it>
In reply to#1546996
On Sat, 2016-12-24 at 02:06 +0100, David Dillow wrote:
>On Sat, 2016-12-24 at 00:00 +0100, Thomas Preisner wrote:
>> diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
>> index a0cacbe..9a3ab58 100644
>> --- a/drivers/net/ethernet/3com/typhoon.c
>> +++ b/drivers/net/ethernet/3com/typhoon.c
>> @@ -2404,6 +2404,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  
>>  	if(!is_valid_ether_addr(dev->dev_addr)) {
>>  		err_msg = "Could not obtain valid ethernet address, aborting";
>> +		err = -EIO;
>>  		goto error_out_reset;
>
>The change above is fine, but the other two should use the return value
>from the failing function call.
>
>
>> @@ -2413,6 +2414,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
>>  	if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
>>  		err_msg = "Could not get Sleep Image version";
>> +		err = -EIO;
>>  		goto error_out_reset;
>>  	}
>>  
>> @@ -2455,6 +2457,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  
>>  	if(register_netdev(dev) < 0) {
>>  		err_msg = "unable to register netdev";
>> +		err = -EIO;
>>  		goto error_out_reset;
>>  	}
>>  

You are of course right. After you mentioning this we've looked into it a bit
further and realized that the return values of failing function calls are not
being used in various occasions inside of typhoon_init_one().
That's why we've created a second patch to fix this misbehavior (if it is one).
In case this was intended, feel free to ignore the second patch.

Patch 1:
Makes the function typhoon_init_one() return a negative error code instead of 0.

Patch 2 [Optional]:
Makes the function typhoon_init_one() return the return value of the
corresponding failing function calls instead of a "fixed" negative error code.

With regards (and merry christmas),
Milan and Thomas

[toc] | [prev] | [next] | [standalone]

#1547052 — [PATCH v2 2/2] drivers: net: ethernet: 3com: fix return value

FromThomas Preisner <thomas.preisner+linux@fau.de>
Date2016-12-24 13:10 +0100
Subject[PATCH v2 2/2] drivers: net: ethernet: 3com: fix return value
Message-ID<sRTjb-6KI-13@gated-at.bofh.it>
In reply to#1547051
In some cases the return value of a failing function is not being used
and the function typhoon_init_one() returns another negative error
code instead.

Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
---
 drivers/net/ethernet/3com/typhoon.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index c88b88a..8821a24 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2370,9 +2370,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 * 4) Get the hardware address.
 	 * 5) Put the card to sleep.
 	 */
-	if (typhoon_reset(ioaddr, WaitSleep) < 0) {
+	err = typhoon_reset(ioaddr, WaitSleep);
+	if (err < 0) {
 		err_msg = "could not reset 3XP";
-		err = -EIO;
 		goto error_out_dma;
 	}
 
@@ -2386,16 +2386,16 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	typhoon_init_interface(tp);
 	typhoon_init_rings(tp);
 
-	if(typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST) < 0) {
+	err = typhoon_boot_3XP(tp, TYPHOON_STATUS_WAITING_FOR_HOST);
+	if(err < 0) {
 		err_msg = "cannot boot 3XP sleep image";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
 	INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_MAC_ADDRESS);
-	if(typhoon_issue_command(tp, 1, &xp_cmd, 1, xp_resp) < 0) {
+	err = typhoon_issue_command(tp, 1, &xp_cmd, 1, xp_resp);
+	if(err < 0) {
 		err_msg = "cannot read MAC address";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
@@ -2430,9 +2430,9 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if(xp_resp[0].numDesc != 0)
 		tp->capabilities |= TYPHOON_WAKEUP_NEEDS_RESET;
 
-	if(typhoon_sleep(tp, PCI_D3hot, 0) < 0) {
+	err = typhoon_sleep(tp, PCI_D3hot, 0);
+	if(err < 0) {
 		err_msg = "cannot put adapter to sleep";
-		err = -EIO;
 		goto error_out_reset;
 	}
 
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web