| Message ID | 1362918526-26730-1-git-send-email-valentin.ilie@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)