netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next] net: remove useless if check from register_netdevice()
@ 2014-02-13  4:47 Denis Kirjanov
  2014-02-13 16:23 ` Alexei Starovoitov
  0 siblings, 1 reply; 6+ messages in thread
From: Denis Kirjanov @ 2014-02-13  4:47 UTC (permalink / raw)
  To: davem, netdev; +Cc: Denis Kirjanov

remove useless if check from register_netdevice()

Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
v1 -> v2: Fixed identation
---
 net/core/dev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 4ad1b78..21a72ad 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5876,8 +5876,7 @@ int register_netdevice(struct net_device *dev)
 	if (dev->netdev_ops->ndo_init) {
 		ret = dev->netdev_ops->ndo_init(dev);
 		if (ret) {
-			if (ret > 0)
-				ret = -EIO;
+			ret = -EIO;
 			goto out;
 		}
 	}
-- 
1.8.0.2

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

* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
  2014-02-13  4:47 [PATCH v2 net-next] net: remove useless if check from register_netdevice() Denis Kirjanov
@ 2014-02-13 16:23 ` Alexei Starovoitov
  2014-02-13 17:15   ` Denis Kirjanov
  2014-02-13 19:21   ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2014-02-13 16:23 UTC (permalink / raw)
  To: Denis Kirjanov; +Cc: David S. Miller, netdev

On Wed, Feb 12, 2014 at 8:47 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> remove useless if check from register_netdevice()
>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
> v1 -> v2: Fixed identation
> ---
>  net/core/dev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4ad1b78..21a72ad 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5876,8 +5876,7 @@ int register_netdevice(struct net_device *dev)
>         if (dev->netdev_ops->ndo_init) {
>                 ret = dev->netdev_ops->ndo_init(dev);
>                 if (ret) {
> -                       if (ret > 0)
> -                               ret = -EIO;
> +                       ret = -EIO;
>                         goto out;

why is it a useless check?
seems perfectly valid to me.
most of the time ndo_init() returns negative error code like -ENOMEM
which we want
to propagate further down and want to override it to -EIO if it's > 0

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

* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
  2014-02-13 16:23 ` Alexei Starovoitov
@ 2014-02-13 17:15   ` Denis Kirjanov
  2014-02-13 18:28     ` Cong Wang
  2014-02-13 19:20     ` David Miller
  2014-02-13 19:21   ` David Miller
  1 sibling, 2 replies; 6+ messages in thread
From: Denis Kirjanov @ 2014-02-13 17:15 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: David S. Miller, netdev

All of the users of ndo_init return negative error code on error path.
Only staging stuff tries to use positive codes like octeon ethernet
which does request_irq inside ndo_open...

On 2/13/14, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Wed, Feb 12, 2014 at 8:47 PM, Denis Kirjanov <kda@linux-powerpc.org>
> wrote:
>> remove useless if check from register_netdevice()
>>
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>> ---
>> v1 -> v2: Fixed identation
>> ---
>>  net/core/dev.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4ad1b78..21a72ad 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5876,8 +5876,7 @@ int register_netdevice(struct net_device *dev)
>>         if (dev->netdev_ops->ndo_init) {
>>                 ret = dev->netdev_ops->ndo_init(dev);
>>                 if (ret) {
>> -                       if (ret > 0)
>> -                               ret = -EIO;
>> +                       ret = -EIO;
>>                         goto out;
>
> why is it a useless check?
> seems perfectly valid to me.
> most of the time ndo_init() returns negative error code like -ENOMEM
> which we want
> to propagate further down and want to override it to -EIO if it's > 0
>

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

* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
  2014-02-13 17:15   ` Denis Kirjanov
@ 2014-02-13 18:28     ` Cong Wang
  2014-02-13 19:20     ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: Cong Wang @ 2014-02-13 18:28 UTC (permalink / raw)
  To: Denis Kirjanov; +Cc: Alexei Starovoitov, David S. Miller, netdev

On Thu, Feb 13, 2014 at 9:15 AM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
> All of the users of ndo_init return negative error code on error path.
> Only staging stuff tries to use positive codes like octeon ethernet
> which does request_irq inside ndo_open...
>

You should put this explanation in the changelog.

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

* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
  2014-02-13 17:15   ` Denis Kirjanov
  2014-02-13 18:28     ` Cong Wang
@ 2014-02-13 19:20     ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2014-02-13 19:20 UTC (permalink / raw)
  To: kda; +Cc: alexei.starovoitov, netdev

From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Thu, 13 Feb 2014 21:15:46 +0400

> All of the users of ndo_init return negative error code on error path.
> Only staging stuff tries to use positive codes like octeon ethernet
> which does request_irq inside ndo_open...

Do not top-post, please.

Quote the necessary material of the person you are replying to, then
provide your response underneath.

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

* Re: [PATCH v2 net-next] net: remove useless if check from register_netdevice()
  2014-02-13 16:23 ` Alexei Starovoitov
  2014-02-13 17:15   ` Denis Kirjanov
@ 2014-02-13 19:21   ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2014-02-13 19:21 UTC (permalink / raw)
  To: alexei.starovoitov; +Cc: kda, netdev

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Thu, 13 Feb 2014 08:23:08 -0800

> On Wed, Feb 12, 2014 at 8:47 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
>> remove useless if check from register_netdevice()
>>
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>> ---
>> v1 -> v2: Fixed identation
>> ---
>>  net/core/dev.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4ad1b78..21a72ad 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -5876,8 +5876,7 @@ int register_netdevice(struct net_device *dev)
>>         if (dev->netdev_ops->ndo_init) {
>>                 ret = dev->netdev_ops->ndo_init(dev);
>>                 if (ret) {
>> -                       if (ret > 0)
>> -                               ret = -EIO;
>> +                       ret = -EIO;
>>                         goto out;
> 
> why is it a useless check?
> seems perfectly valid to me.
> most of the time ndo_init() returns negative error code like -ENOMEM
> which we want
> to propagate further down and want to override it to -EIO if it's > 0

Agreed, and if it is useless then unconditionally returining -EIO is
absolutely the wrong thing to do.  We should always use whatever negative
error code ndo_init() gave us.

I'm not applying this patch.

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

end of thread, other threads:[~2014-02-13 19:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-13  4:47 [PATCH v2 net-next] net: remove useless if check from register_netdevice() Denis Kirjanov
2014-02-13 16:23 ` Alexei Starovoitov
2014-02-13 17:15   ` Denis Kirjanov
2014-02-13 18:28     ` Cong Wang
2014-02-13 19:20     ` David Miller
2014-02-13 19:21   ` David Miller

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).