netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: can: af_can.c: Fix checkpatch warnings
@ 2013-03-10 12:28 Valentin Ilie
  2013-03-10 12:45 ` Oliver Hartkopp
  2013-03-10 21:15 ` [PATCHv2] " Valentin Ilie
  0 siblings, 2 replies; 7+ messages in thread
From: Valentin Ilie @ 2013-03-10 12:28 UTC (permalink / raw)
  To: socketcan, davem; +Cc: netdev, linux-kernel, Valentin Ilie

Replace printk(KERN_ERR with pr_err
Add space before {

Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>
---
 net/can/af_can.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index c48e522..a14c0aa 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -525,7 +525,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 
 	d = find_dev_rcv_lists(dev);
 	if (!d) {
-		printk(KERN_ERR "BUG: receive list not found for "
+		pr_err("BUG: receive list not found for "
 		       "dev %s, id %03X, mask %03X\n",
 		       DNAME(dev), can_id, mask);
 		goto out;
@@ -552,7 +552,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 	 */
 
 	if (!r) {
-		printk(KERN_ERR "BUG: receive list entry not found for "
+		pr_err("BUG: receive list entry not found for "
 		       "dev %s, id %03X, mask %03X\n",
 		       DNAME(dev), can_id, mask);
 		r = NULL;
@@ -749,7 +749,7 @@ int can_proto_register(const struct can_proto *cp)
 	int err = 0;
 
 	if (proto < 0 || proto >= CAN_NPROTO) {
-		printk(KERN_ERR "can: protocol number %d out of range\n",
+		pr_err("can: protocol number %d out of range\n",
 		       proto);
 		return -EINVAL;
 	}
@@ -761,7 +761,7 @@ int can_proto_register(const struct can_proto *cp)
 	mutex_lock(&proto_tab_lock);
 
 	if (proto_tab[proto]) {
-		printk(KERN_ERR "can: protocol %d already registered\n",
+		pr_err("can: protocol %d already registered\n",
 		       proto);
 		err = -EBUSY;
 	} else
@@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 		/* create new dev_rcv_lists for this device */
 		d = kzalloc(sizeof(*d), GFP_KERNEL);
 		if (!d) {
-			printk(KERN_ERR
-			       "can: allocation of receive list failed\n");
+			pr_err("can: allocation of receive list failed\n");
 			return NOTIFY_DONE;
 		}
 		BUG_ON(dev->ml_priv);
@@ -838,7 +837,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 				dev->ml_priv = NULL;
 			}
 		} else
-			printk(KERN_ERR "can: notifier: receive list not "
+			pr_err("can: notifier: receive list not "
 			       "found for dev %s\n", dev->name);
 
 		spin_unlock(&can_rcvlists_lock);
@@ -927,7 +926,7 @@ static __exit void can_exit(void)
 	/* remove created dev_rcv_lists from still registered CAN devices */
 	rcu_read_lock();
 	for_each_netdev_rcu(&init_net, dev) {
-		if (dev->type == ARPHRD_CAN && dev->ml_priv){
+		if (dev->type == ARPHRD_CAN && dev->ml_priv) {
 
 			struct dev_rcv_lists *d = dev->ml_priv;
 
-- 
1.8.1.2

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

* Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
  2013-03-10 12:28 [PATCH] net: can: af_can.c: Fix checkpatch warnings Valentin Ilie
@ 2013-03-10 12:45 ` Oliver Hartkopp
  2013-03-10 13:05   ` Daniel Mack
  2013-03-10 21:15 ` [PATCHv2] " Valentin Ilie
  1 sibling, 1 reply; 7+ messages in thread
From: Oliver Hartkopp @ 2013-03-10 12:45 UTC (permalink / raw)
  To: Valentin Ilie; +Cc: davem, netdev, linux-kernel

Hallo Valentin,

thanks for the idea for the cleanup.

But if you replace the the printk() statements it's worth to check if code
fits into a single line now, e.g.

On 10.03.2013 13:28, Valentin Ilie wrote:

 		r = NULL;
> @@ -749,7 +749,7 @@ int can_proto_register(const struct can_proto *cp)
>  	int err = 0;
>  
>  	if (proto < 0 || proto >= CAN_NPROTO) {
> -		printk(KERN_ERR "can: protocol number %d out of range\n",
> +		pr_err("can: protocol number %d out of range\n",
>  		       proto);


here " ,proto);" can obviously be move into the previous line while preserving
the 80 chars per line rule.


>  		return -EINVAL;
>  	}
> @@ -761,7 +761,7 @@ int can_proto_register(const struct can_proto *cp)
>  	mutex_lock(&proto_tab_lock);
>  
>  	if (proto_tab[proto]) {
> -		printk(KERN_ERR "can: protocol %d already registered\n",
> +		pr_err("can: protocol %d already registered\n",
>  		       proto);


here too

>  		err = -EBUSY;
>  	} else
> @@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>  		/* create new dev_rcv_lists for this device */
>  		d = kzalloc(sizeof(*d), GFP_KERNEL);
>  		if (!d) {
> -			printk(KERN_ERR
> -			       "can: allocation of receive list failed\n");
> +			pr_err("can: allocation of receive list failed\n");
>  			return NOTIFY_DONE;
>  		}


As i've seen in former patches from Joe Perches OOM messages are obsolete.

You could write

if(!d)
	return NOTIFY_DONE;

here instead.

Thanks,
Oliver

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

* Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
  2013-03-10 12:45 ` Oliver Hartkopp
@ 2013-03-10 13:05   ` Daniel Mack
  2013-03-10 13:11     ` Oliver Hartkopp
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Mack @ 2013-03-10 13:05 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: Valentin Ilie, davem, netdev, linux-kernel

On Sun, Mar 10, 2013 at 1:45 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hallo Valentin,
>
> thanks for the idea for the cleanup.
>
> But if you replace the the printk() statements it's worth to check if code
> fits into a single line now, e.g.
>
> On 10.03.2013 13:28, Valentin Ilie wrote:
>
>                 r = NULL;
>> @@ -749,7 +749,7 @@ int can_proto_register(const struct can_proto *cp)
>>       int err = 0;
>>
>>       if (proto < 0 || proto >= CAN_NPROTO) {
>> -             printk(KERN_ERR "can: protocol number %d out of range\n",
>> +             pr_err("can: protocol number %d out of range\n",
>>                      proto);
>
>
> here " ,proto);" can obviously be move into the previous line while preserving
> the 80 chars per line rule.
>
>
>>               return -EINVAL;
>>       }
>> @@ -761,7 +761,7 @@ int can_proto_register(const struct can_proto *cp)
>>       mutex_lock(&proto_tab_lock);
>>
>>       if (proto_tab[proto]) {
>> -             printk(KERN_ERR "can: protocol %d already registered\n",
>> +             pr_err("can: protocol %d already registered\n",
>>                      proto);
>
>
> here too
>
>>               err = -EBUSY;
>>       } else
>> @@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>>               /* create new dev_rcv_lists for this device */
>>               d = kzalloc(sizeof(*d), GFP_KERNEL);
>>               if (!d) {
>> -                     printk(KERN_ERR
>> -                            "can: allocation of receive list failed\n");
>> +                     pr_err("can: allocation of receive list failed\n");
>>                       return NOTIFY_DONE;
>>               }
>
>
> As i've seen in former patches from Joe Perches OOM messages are obsolete.
>
> You could write
>
> if(!d)
>         return NOTIFY_DONE;
>
> here instead.

While at it, why not switch to dev_err() instead?


Daniel

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

* Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
  2013-03-10 13:05   ` Daniel Mack
@ 2013-03-10 13:11     ` Oliver Hartkopp
  0 siblings, 0 replies; 7+ messages in thread
From: Oliver Hartkopp @ 2013-03-10 13:11 UTC (permalink / raw)
  To: Daniel Mack; +Cc: Valentin Ilie, davem, netdev, linux-kernel

On 10.03.2013 14:05, Daniel Mack wrote:


>>> @@ -817,8 +817,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>>>               /* create new dev_rcv_lists for this device */
>>>               d = kzalloc(sizeof(*d), GFP_KERNEL);
>>>               if (!d) {
>>> -                     printk(KERN_ERR
>>> -                            "can: allocation of receive list failed\n");
>>> +                     pr_err("can: allocation of receive list failed\n");
>>>                       return NOTIFY_DONE;
>>>               }
>>
>>
>> As i've seen in former patches from Joe Perches OOM messages are obsolete.
>>
>> You could write
>>
>> if(!d)
>>         return NOTIFY_DONE;
>>
>> here instead.
> 
> While at it, why not switch to dev_err() instead?


Hm, no.

1. pr_err() would be fine, as dev_err() is for drivers - and we're in
linux/net here and not in linux/drivers

2. When removing the entire printk() statement there's nothing to be modified left

Regards,
Oliver

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

* [PATCHv2] net: can: af_can.c: Fix checkpatch warnings
  2013-03-10 12:28 [PATCH] net: can: af_can.c: Fix checkpatch warnings Valentin Ilie
  2013-03-10 12:45 ` Oliver Hartkopp
@ 2013-03-10 21:15 ` Valentin Ilie
  2013-03-11  7:02   ` Oliver Hartkopp
  1 sibling, 1 reply; 7+ messages in thread
From: Valentin Ilie @ 2013-03-10 21:15 UTC (permalink / raw)
  To: socketcan, davem; +Cc: linux-can, netdev, linux-kernel, Valentin Ilie

Replace printk(KERN_ERR with pr_err
Add space before {
Removed OOM messages

Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>
---
 net/can/af_can.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index c48e522..8bacf28 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -525,7 +525,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 
 	d = find_dev_rcv_lists(dev);
 	if (!d) {
-		printk(KERN_ERR "BUG: receive list not found for "
+		pr_err("BUG: receive list not found for "
 		       "dev %s, id %03X, mask %03X\n",
 		       DNAME(dev), can_id, mask);
 		goto out;
@@ -552,7 +552,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 	 */
 
 	if (!r) {
-		printk(KERN_ERR "BUG: receive list entry not found for "
+		pr_err("BUG: receive list entry not found for "
 		       "dev %s, id %03X, mask %03X\n",
 		       DNAME(dev), can_id, mask);
 		r = NULL;
@@ -749,8 +749,7 @@ int can_proto_register(const struct can_proto *cp)
 	int err = 0;
 
 	if (proto < 0 || proto >= CAN_NPROTO) {
-		printk(KERN_ERR "can: protocol number %d out of range\n",
-		       proto);
+		pr_err("can: protocol number %d out of range\n", proto);
 		return -EINVAL;
 	}
 
@@ -761,8 +760,7 @@ int can_proto_register(const struct can_proto *cp)
 	mutex_lock(&proto_tab_lock);
 
 	if (proto_tab[proto]) {
-		printk(KERN_ERR "can: protocol %d already registered\n",
-		       proto);
+		pr_err("can: protocol %d already registered\n", proto);
 		err = -EBUSY;
 	} else
 		RCU_INIT_POINTER(proto_tab[proto], cp);
@@ -816,11 +814,8 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 
 		/* create new dev_rcv_lists for this device */
 		d = kzalloc(sizeof(*d), GFP_KERNEL);
-		if (!d) {
-			printk(KERN_ERR
-			       "can: allocation of receive list failed\n");
+		if (!d)
 			return NOTIFY_DONE;
-		}
 		BUG_ON(dev->ml_priv);
 		dev->ml_priv = d;
 
@@ -838,8 +833,8 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 				dev->ml_priv = NULL;
 			}
 		} else
-			printk(KERN_ERR "can: notifier: receive list not "
-			       "found for dev %s\n", dev->name);
+			pr_err("can: notifier: receive list not found for dev "
+			       "%s\n", dev->name);
 
 		spin_unlock(&can_rcvlists_lock);
 
@@ -927,7 +922,7 @@ static __exit void can_exit(void)
 	/* remove created dev_rcv_lists from still registered CAN devices */
 	rcu_read_lock();
 	for_each_netdev_rcu(&init_net, dev) {
-		if (dev->type == ARPHRD_CAN && dev->ml_priv){
+		if (dev->type == ARPHRD_CAN && dev->ml_priv) {
 
 			struct dev_rcv_lists *d = dev->ml_priv;
 
-- 
1.8.1.2


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

* Re: [PATCHv2] net: can: af_can.c: Fix checkpatch warnings
  2013-03-10 21:15 ` [PATCHv2] " Valentin Ilie
@ 2013-03-11  7:02   ` Oliver Hartkopp
  2013-03-11 11:40     ` David Miller
  0 siblings, 1 reply; 7+ messages in thread
From: Oliver Hartkopp @ 2013-03-11  7:02 UTC (permalink / raw)
  To: Valentin Ilie; +Cc: davem, linux-can, netdev, linux-kernel

On 10.03.2013 22:15, Valentin Ilie wrote:

> Replace printk(KERN_ERR with pr_err
> Add space before {
> Removed OOM messages
> 
> Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>


Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Thanks Valentin


> ---
>  net/can/af_can.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index c48e522..8bacf28 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -525,7 +525,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
>  
>  	d = find_dev_rcv_lists(dev);
>  	if (!d) {
> -		printk(KERN_ERR "BUG: receive list not found for "
> +		pr_err("BUG: receive list not found for "
>  		       "dev %s, id %03X, mask %03X\n",
>  		       DNAME(dev), can_id, mask);
>  		goto out;
> @@ -552,7 +552,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
>  	 */
>  
>  	if (!r) {
> -		printk(KERN_ERR "BUG: receive list entry not found for "
> +		pr_err("BUG: receive list entry not found for "
>  		       "dev %s, id %03X, mask %03X\n",
>  		       DNAME(dev), can_id, mask);
>  		r = NULL;
> @@ -749,8 +749,7 @@ int can_proto_register(const struct can_proto *cp)
>  	int err = 0;
>  
>  	if (proto < 0 || proto >= CAN_NPROTO) {
> -		printk(KERN_ERR "can: protocol number %d out of range\n",
> -		       proto);
> +		pr_err("can: protocol number %d out of range\n", proto);
>  		return -EINVAL;
>  	}
>  
> @@ -761,8 +760,7 @@ int can_proto_register(const struct can_proto *cp)
>  	mutex_lock(&proto_tab_lock);
>  
>  	if (proto_tab[proto]) {
> -		printk(KERN_ERR "can: protocol %d already registered\n",
> -		       proto);
> +		pr_err("can: protocol %d already registered\n", proto);
>  		err = -EBUSY;
>  	} else
>  		RCU_INIT_POINTER(proto_tab[proto], cp);
> @@ -816,11 +814,8 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>  
>  		/* create new dev_rcv_lists for this device */
>  		d = kzalloc(sizeof(*d), GFP_KERNEL);
> -		if (!d) {
> -			printk(KERN_ERR
> -			       "can: allocation of receive list failed\n");
> +		if (!d)
>  			return NOTIFY_DONE;
> -		}
>  		BUG_ON(dev->ml_priv);
>  		dev->ml_priv = d;
>  
> @@ -838,8 +833,8 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
>  				dev->ml_priv = NULL;
>  			}
>  		} else
> -			printk(KERN_ERR "can: notifier: receive list not "
> -			       "found for dev %s\n", dev->name);
> +			pr_err("can: notifier: receive list not found for dev "
> +			       "%s\n", dev->name);
>  
>  		spin_unlock(&can_rcvlists_lock);
>  
> @@ -927,7 +922,7 @@ static __exit void can_exit(void)
>  	/* remove created dev_rcv_lists from still registered CAN devices */
>  	rcu_read_lock();
>  	for_each_netdev_rcu(&init_net, dev) {
> -		if (dev->type == ARPHRD_CAN && dev->ml_priv){
> +		if (dev->type == ARPHRD_CAN && dev->ml_priv) {
>  
>  			struct dev_rcv_lists *d = dev->ml_priv;
>  



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

* Re: [PATCHv2] net: can: af_can.c: Fix checkpatch warnings
  2013-03-11  7:02   ` Oliver Hartkopp
@ 2013-03-11 11:40     ` David Miller
  0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2013-03-11 11:40 UTC (permalink / raw)
  To: socketcan; +Cc: valentin.ilie, linux-can, netdev, linux-kernel

From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Mon, 11 Mar 2013 08:02:43 +0100

> On 10.03.2013 22:15, Valentin Ilie wrote:
> 
>> Replace printk(KERN_ERR with pr_err
>> Add space before {
>> Removed OOM messages
>> 
>> Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>
> 
> 
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Applied, thanks.

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

end of thread, other threads:[~2013-03-11 11:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-10 12:28 [PATCH] net: can: af_can.c: Fix checkpatch warnings Valentin Ilie
2013-03-10 12:45 ` Oliver Hartkopp
2013-03-10 13:05   ` Daniel Mack
2013-03-10 13:11     ` Oliver Hartkopp
2013-03-10 21:15 ` [PATCHv2] " Valentin Ilie
2013-03-11  7:02   ` Oliver Hartkopp
2013-03-11 11:40     ` 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).