All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Valentin Ilie <valentin.ilie@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: can: af_can.c: Fix checkpatch warnings
Date: Sun, 10 Mar 2013 13:45:22 +0100	[thread overview]
Message-ID: <513C8062.6030001@hartkopp.net> (raw)
In-Reply-To: <1362918526-26730-1-git-send-email-valentin.ilie@gmail.com>

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

  reply	other threads:[~2013-03-10 12:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10 12:28 [PATCH] net: can: af_can.c: Fix checkpatch warnings Valentin Ilie
2013-03-10 12:45 ` Oliver Hartkopp [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=513C8062.6030001@hartkopp.net \
    --to=socketcan@hartkopp.net \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=valentin.ilie@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.