All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Morris <ipm@chirality.org.uk>
To: Joe Perches <joe@perches.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net-next 0/2] ipv6: coding style - comparisons with NULL
Date: Tue, 31 Mar 2015 17:39:00 +0100	[thread overview]
Message-ID: <551ACDA4.8000302@chirality.org.uk> (raw)
In-Reply-To: <1427650577.2715.32.camel@perches.com>

Hello Joe,

Many thanks for your invaluable feedback, as ever!

I was aware this one has the potential to be a somewhat controversial 
topic and whilst my analysis was not quite as scientific as yours I too 
had detected the preference for the shorter form hence the reason for 
adopting that style : however your stats are a great illustration so 
thanks for putting them forward.

I guess the thing with coding style is precisely that : it's a style 
rather than something that is correct or not but my perspective (FWIW, 
probably not that much!) is that we should at least try to be consistent 
within an area of code. As such I can totally understand one area using 
NULL and another not given the size of the kernel as a whole but I 
thought it worth trying to align stuff within IPV6.

IPV6 is a particular interest for me but I'd be happy to cook up a 
similar patch for IPV4 if there's interest.

Thanks again,

Ian.

On 29/03/15 18:36, Joe Perches wrote:
> On Sun, 2015-03-29 at 14:00 +0100, Ian Morris wrote:
>> The IPV6 code uses multiple different styles when comparing with NULL
>> (I.e. x == NULL and !x as well as x != NULL and x). Generally the
>> latter form is preferred according to checkpatch and so this changes
>> aligns the code to this style.
> Hello Ian.
>
> While I prefer this style, it can be a "maintainer's choice"
> preference and this might be a bit contentious to some.
>
> It might be nice to show the relative counts of each form
> before this change to show the relative counts use of each
> form to help establish consensus for the value for this patch.
>
> (Or David could just apply it instead:)
>
> Using coccinelle to check for pointer comparisons with and
> without NULL for net/ipv6/*.[ch]:
>
> with NULL:	260
> without NULL:	779
>
> So there was a 3:1 preference for the bare form.
>
> (and for net/ipv4/*.[ch] it's closer to 4:1)
>
> with NULL:	276
> without NULL:	914
>
> (trivial cocci script below)
> @@
> type A;
> A *b;
> @@
>
> * b == NULL
>
> @@
> type A;
> A *b;
> @@
>
> * b != NULL
>
>
>

  reply	other threads:[~2015-03-31 16:39 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-29 13:00 [PATCH net-next 0/2] ipv6: coding style - comparisons with NULL Ian Morris
2015-03-29 13:00 ` [PATCH net-next 1/2] ipv6: coding style: comparison for equality " Ian Morris
2015-03-29 13:00 ` [PATCH net-next 2/2] ipv6: coding style: comparison for inequality " Ian Morris
2015-03-29 17:36 ` [PATCH net-next 0/2] ipv6: coding style - comparisons " Joe Perches
2015-03-31 16:39   ` Ian Morris [this message]
2015-03-31 17:50     ` Joe Perches
2015-03-31 17:53 ` David Miller
     [not found]   ` <1427825806.10376.42.camel@perches.com>
2015-03-31 18:40     ` [PATCH] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 72 columns Joe Perches
2015-03-31 19:06       ` David Miller
2015-04-01 19:20       ` Andrew Morton
2015-04-01 19:28         ` Joe Perches
2015-04-01 19:36         ` Jonathan Corbet
2015-04-01 20:09           ` Joe Perches
2015-04-01 20:18             ` Andrew Morton
2015-04-01 20:33               ` [PATCH V2] checkpatch/SubmittingPatches: Suggest line wrapping commit messages at 75 columns Joe Perches

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=551ACDA4.8000302@chirality.org.uk \
    --to=ipm@chirality.org.uk \
    --cc=joe@perches.com \
    --cc=netdev@vger.kernel.org \
    /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.