* [PATCH] Staging: gdm72xx: fix coding style
@ 2014-09-29 18:15 Thibaut Robert
2014-09-29 18:31 ` Joe Perches
0 siblings, 1 reply; 7+ messages in thread
From: Thibaut Robert @ 2014-09-29 18:15 UTC (permalink / raw)
To: Greg Kroah-Hartman, Michalis Pappas, Davide Gianforte
Cc: devel, linux-kernel, Thibaut Robert
This patch remove a checkstyle warning in netlink_k.c.
Note: This is my homework of the day.
Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
---
drivers/staging/gdm72xx/netlink_k.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
index 9bf00e6..0a5831a 100644
--- a/drivers/staging/gdm72xx/netlink_k.c
+++ b/drivers/staging/gdm72xx/netlink_k.c
@@ -147,12 +147,11 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
if (!ret) {
return len;
- } else {
- if (ret != -ESRCH) {
- pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
- group, type, len, ret);
- }
- ret = 0;
}
- return ret;
+
+ if (ret != -ESRCH) {
+ pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
+ group, type, len, ret);
+ }
+ return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Staging: gdm72xx: fix coding style
2014-09-29 18:15 [PATCH] Staging: gdm72xx: fix coding style Thibaut Robert
@ 2014-09-29 18:31 ` Joe Perches
2014-09-30 10:20 ` Thibaut Robert
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Joe Perches @ 2014-09-29 18:31 UTC (permalink / raw)
To: Thibaut Robert
Cc: Greg Kroah-Hartman, Michalis Pappas, Davide Gianforte, devel,
linux-kernel
On Mon, 2014-09-29 at 20:15 +0200, Thibaut Robert wrote:
> This patch remove a checkstyle warning in netlink_k.c.
> Note: This is my homework of the day.
The "Note: " line isn't useful for a commit message.
Neither is the checkpatch warning bit.
Maybe the commit message could be something like:
"Remove unnecessary else after return".
> diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
[]
> @@ -147,12 +147,11 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
>
> if (!ret) {
> return len;
> - } else {
> - if (ret != -ESRCH) {
> - pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> - group, type, len, ret);
> - }
> - ret = 0;
> }
> - return ret;
> +
> + if (ret != -ESRCH) {
> + pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> + group, type, len, ret);
> + }
OK, but you don't need the braces around the pr_err.
My preference is to also fix the typo in the function
and would be something like:
---
drivers/staging/gdm72xx/netlink_k.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
index 9bf00e6..43d2a49 100644
--- a/drivers/staging/gdm72xx/netlink_k.c
+++ b/drivers/staging/gdm72xx/netlink_k.c
@@ -120,9 +120,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
struct nlmsghdr *nlh;
int ret = 0;
- if (group > ND_MAX_GROUP) {
- pr_err("Group %d is invalied.\n", group);
- pr_err("Valid group is 0 ~ %d.\n", ND_MAX_GROUP);
+ if (group < 0 || group > ND_MAX_GROUP) {
+ pr_err("Invalid group %d (valid groups are 0 to %d)\n",
+ group, ND_MAX_GROUP);
return -EINVAL;
}
@@ -144,15 +144,12 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
NETLINK_CB(skb).dst_group = 0;
ret = netlink_broadcast(sock, skb, 0, group+1, GFP_ATOMIC);
-
- if (!ret) {
+ if (!ret)
return len;
- } else {
- if (ret != -ESRCH) {
- pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
- group, type, len, ret);
- }
- ret = 0;
- }
- return ret;
+
+ if (ret != -ESRCH)
+ pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
+ group, type, len, ret);
+
+ return 0;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Staging: gdm72xx: fix coding style
2014-09-29 18:31 ` Joe Perches
@ 2014-09-30 10:20 ` Thibaut Robert
2014-09-30 10:23 ` Thibaut Robert
2014-09-30 11:03 ` [PATCH] staging: gdm72xx: fix typo and " Thibaut Robert
2 siblings, 0 replies; 7+ messages in thread
From: Thibaut Robert @ 2014-09-30 10:20 UTC (permalink / raw)
To: Joe Perches
Cc: Greg Kroah-Hartman, Michalis Pappas, Davide Gianforte, devel,
linux-kernel
Le Monday 29 Sep 2014 à 11:31:24 (-0700), Joe Perches a écrit :
> On Mon, 2014-09-29 at 20:15 +0200, Thibaut Robert wrote:
> > This patch remove a checkstyle warning in netlink_k.c.
> > Note: This is my homework of the day.
>
> The "Note: " line isn't useful for a commit message.
> Neither is the checkpatch warning bit.
>
> Maybe the commit message could be something like:
> "Remove unnecessary else after return".
>
> > diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
> []
> > @@ -147,12 +147,11 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> >
> > if (!ret) {
> > return len;
> > - } else {
> > - if (ret != -ESRCH) {
> > - pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> > - group, type, len, ret);
> > - }
> > - ret = 0;
> > }
> > - return ret;
> > +
> > + if (ret != -ESRCH) {
> > + pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> > + group, type, len, ret);
> > + }
>
> OK, but you don't need the braces around the pr_err.
>
> My preference is to also fix the typo in the function
> and would be something like:
> ---
> drivers/staging/gdm72xx/netlink_k.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
> index 9bf00e6..43d2a49 100644
> --- a/drivers/staging/gdm72xx/netlink_k.c
> +++ b/drivers/staging/gdm72xx/netlink_k.c
> @@ -120,9 +120,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> struct nlmsghdr *nlh;
> int ret = 0;
>
> - if (group > ND_MAX_GROUP) {
> - pr_err("Group %d is invalied.\n", group);
> - pr_err("Valid group is 0 ~ %d.\n", ND_MAX_GROUP);
> + if (group < 0 || group > ND_MAX_GROUP) {
> + pr_err("Invalid group %d (valid groups are 0 to %d)\n",
> + group, ND_MAX_GROUP);
> return -EINVAL;
> }
>
> @@ -144,15 +144,12 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> NETLINK_CB(skb).dst_group = 0;
>
> ret = netlink_broadcast(sock, skb, 0, group+1, GFP_ATOMIC);
> -
> - if (!ret) {
> + if (!ret)
> return len;
> - } else {
> - if (ret != -ESRCH) {
> - pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> - group, type, len, ret);
> - }
> - ret = 0;
> - }
> - return ret;
> +
> + if (ret != -ESRCH)
> + pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> + group, type, len, ret);
> +
> + return 0;
> }
>
>
>
Thanks for this newbie friendly attitude. That's much appreciated. An updated patch will follow.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Staging: gdm72xx: fix coding style
2014-09-29 18:31 ` Joe Perches
2014-09-30 10:20 ` Thibaut Robert
@ 2014-09-30 10:23 ` Thibaut Robert
2014-09-30 11:02 ` Thibaut Robert
2014-09-30 11:03 ` [PATCH] staging: gdm72xx: fix typo and " Thibaut Robert
2 siblings, 1 reply; 7+ messages in thread
From: Thibaut Robert @ 2014-09-30 10:23 UTC (permalink / raw)
To: Joe Perches
Cc: Greg Kroah-Hartman, Michalis Pappas, Davide Gianforte, devel,
linux-kernel, Thibaut Robert
Remove unnecessary else after return.
Follow coding style for curly braces around single statements.
Fix a typo in error message.
Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
---
drivers/staging/gdm72xx/netlink_k.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
index 9bf00e6..31862e5 100644
--- a/drivers/staging/gdm72xx/netlink_k.c
+++ b/drivers/staging/gdm72xx/netlink_k.c
@@ -71,10 +71,11 @@ static void netlink_rcv_cb(struct sk_buff *skb)
if (dev) {
rcv_cb(dev, nlh->nlmsg_type, msg, mlen);
dev_put(dev);
- } else
+ } else {
netdev_err(skb->dev,
"dev_get_by_index(%d) is not found.\n",
ifindex);
+ }
} else {
netdev_err(skb->dev, "Unregistered Callback\n");
}
@@ -120,9 +121,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
struct nlmsghdr *nlh;
int ret = 0;
- if (group > ND_MAX_GROUP) {
- pr_err("Group %d is invalied.\n", group);
- pr_err("Valid group is 0 ~ %d.\n", ND_MAX_GROUP);
+ if (group < 0 || group > ND_MAX_GROUP) {
+ pr_err("Invalid group %d (valid groups are 0 to %d)\n",
+ group, ND_MAX_GROUP);
return -EINVAL;
}
@@ -145,14 +146,11 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
ret = netlink_broadcast(sock, skb, 0, group+1, GFP_ATOMIC);
- if (!ret) {
+ if (!ret)
return len;
- } else {
- if (ret != -ESRCH) {
- pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
- group, type, len, ret);
- }
- ret = 0;
- }
- return ret;
+
+ if (ret != -ESRCH)
+ pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
+ group, type, len, ret);
+ return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Staging: gdm72xx: fix coding style
2014-09-30 10:23 ` Thibaut Robert
@ 2014-09-30 11:02 ` Thibaut Robert
0 siblings, 0 replies; 7+ messages in thread
From: Thibaut Robert @ 2014-09-30 11:02 UTC (permalink / raw)
To: Joe Perches
Cc: Greg Kroah-Hartman, Michalis Pappas, Davide Gianforte, devel,
linux-kernel
Le Tuesday 30 Sep 2014 à 12:23:12 (+0200), Thibaut Robert a écrit :
> Remove unnecessary else after return.
> Follow coding style for curly braces around single statements.
> Fix a typo in error message.
>
> Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
> ---
> drivers/staging/gdm72xx/netlink_k.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
> index 9bf00e6..31862e5 100644
> --- a/drivers/staging/gdm72xx/netlink_k.c
> +++ b/drivers/staging/gdm72xx/netlink_k.c
> @@ -71,10 +71,11 @@ static void netlink_rcv_cb(struct sk_buff *skb)
> if (dev) {
> rcv_cb(dev, nlh->nlmsg_type, msg, mlen);
> dev_put(dev);
> - } else
> + } else {
> netdev_err(skb->dev,
> "dev_get_by_index(%d) is not found.\n",
> ifindex);
> + }
> } else {
> netdev_err(skb->dev, "Unregistered Callback\n");
> }
> @@ -120,9 +121,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> struct nlmsghdr *nlh;
> int ret = 0;
>
> - if (group > ND_MAX_GROUP) {
> - pr_err("Group %d is invalied.\n", group);
> - pr_err("Valid group is 0 ~ %d.\n", ND_MAX_GROUP);
> + if (group < 0 || group > ND_MAX_GROUP) {
> + pr_err("Invalid group %d (valid groups are 0 to %d)\n",
> + group, ND_MAX_GROUP);
> return -EINVAL;
> }
>
> @@ -145,14 +146,11 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
>
> ret = netlink_broadcast(sock, skb, 0, group+1, GFP_ATOMIC);
>
> - if (!ret) {
> + if (!ret)
> return len;
> - } else {
> - if (ret != -ESRCH) {
> - pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> - group, type, len, ret);
> - }
> - ret = 0;
> - }
> - return ret;
> +
> + if (ret != -ESRCH)
> + pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
> + group, type, len, ret);
> + return 0;
> }
> --
> 1.9.1
>
I've just seen that some identical changes have already been pushed in the meanwhile, the previous patch does not apply. An new patch will follow (mainly for the typo correction)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] staging: gdm72xx: fix typo and coding style
2014-09-29 18:31 ` Joe Perches
2014-09-30 10:20 ` Thibaut Robert
2014-09-30 10:23 ` Thibaut Robert
@ 2014-09-30 11:03 ` Thibaut Robert
2014-10-05 1:18 ` Greg Kroah-Hartman
2 siblings, 1 reply; 7+ messages in thread
From: Thibaut Robert @ 2014-09-30 11:03 UTC (permalink / raw)
To: Joe Perches
Cc: Greg Kroah-Hartman, Michalis Pappas, Davide Gianforte, devel,
linux-kernel, Thibaut Robert
Fix a typo in error message.
Add missing curling braces to conform to coding style.
Improve readibility.
Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
---
drivers/staging/gdm72xx/netlink_k.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
index 9d78bfc..31862e5 100644
--- a/drivers/staging/gdm72xx/netlink_k.c
+++ b/drivers/staging/gdm72xx/netlink_k.c
@@ -71,10 +71,11 @@ static void netlink_rcv_cb(struct sk_buff *skb)
if (dev) {
rcv_cb(dev, nlh->nlmsg_type, msg, mlen);
dev_put(dev);
- } else
+ } else {
netdev_err(skb->dev,
"dev_get_by_index(%d) is not found.\n",
ifindex);
+ }
} else {
netdev_err(skb->dev, "Unregistered Callback\n");
}
@@ -120,9 +121,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
struct nlmsghdr *nlh;
int ret = 0;
- if (group > ND_MAX_GROUP) {
- pr_err("Group %d is invalied.\n", group);
- pr_err("Valid group is 0 ~ %d.\n", ND_MAX_GROUP);
+ if (group < 0 || group > ND_MAX_GROUP) {
+ pr_err("Invalid group %d (valid groups are 0 to %d)\n",
+ group, ND_MAX_GROUP);
return -EINVAL;
}
@@ -147,10 +148,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
if (!ret)
return len;
- if (ret != -ESRCH) {
+
+ if (ret != -ESRCH)
pr_err("netlink_broadcast g=%d, t=%d, l=%d, r=%d\n",
group, type, len, ret);
- }
- ret = 0;
- return ret;
+ return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: gdm72xx: fix typo and coding style
2014-09-30 11:03 ` [PATCH] staging: gdm72xx: fix typo and " Thibaut Robert
@ 2014-10-05 1:18 ` Greg Kroah-Hartman
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2014-10-05 1:18 UTC (permalink / raw)
To: Thibaut Robert; +Cc: Joe Perches, devel, Davide Gianforte, linux-kernel
On Tue, Sep 30, 2014 at 01:03:03PM +0200, Thibaut Robert wrote:
> Fix a typo in error message.
> Add missing curling braces to conform to coding style.
> Improve readibility.
That's three things in one patch :(
Please only do one thing per patch.
>
> Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
> ---
> drivers/staging/gdm72xx/netlink_k.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
> index 9d78bfc..31862e5 100644
> --- a/drivers/staging/gdm72xx/netlink_k.c
> +++ b/drivers/staging/gdm72xx/netlink_k.c
> @@ -71,10 +71,11 @@ static void netlink_rcv_cb(struct sk_buff *skb)
> if (dev) {
> rcv_cb(dev, nlh->nlmsg_type, msg, mlen);
> dev_put(dev);
> - } else
> + } else {
> netdev_err(skb->dev,
> "dev_get_by_index(%d) is not found.\n",
> ifindex);
> + }
> } else {
> netdev_err(skb->dev, "Unregistered Callback\n");
> }
> @@ -120,9 +121,9 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> struct nlmsghdr *nlh;
> int ret = 0;
>
> - if (group > ND_MAX_GROUP) {
> - pr_err("Group %d is invalied.\n", group);
> - pr_err("Valid group is 0 ~ %d.\n", ND_MAX_GROUP);
> + if (group < 0 || group > ND_MAX_GROUP) {
How can group be less than 0?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-05 1:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-29 18:15 [PATCH] Staging: gdm72xx: fix coding style Thibaut Robert
2014-09-29 18:31 ` Joe Perches
2014-09-30 10:20 ` Thibaut Robert
2014-09-30 10:23 ` Thibaut Robert
2014-09-30 11:02 ` Thibaut Robert
2014-09-30 11:03 ` [PATCH] staging: gdm72xx: fix typo and " Thibaut Robert
2014-10-05 1:18 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.