netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: ricknu-0@student.ltu.se
Cc: xiyou.wangcong@gmail.com, linux-kernel@vger.kernel.org,
	herbert@gondor.apana.org.au, akpm@osdl.org,
	netdev@vger.kernel.org
Subject: Re: [Patch] net/xfrm/xfrm_policy.c: Some small improvements
Date: Thu, 06 Dec 2007 19:22:49 -0800 (PST)	[thread overview]
Message-ID: <20071206.192249.193354742.davem@davemloft.net> (raw)
In-Reply-To: <4758093A.7010608@student.ltu.se>

From: Richard Knutsson <ricknu-0@student.ltu.se>
Date: Thu, 06 Dec 2007 15:37:46 +0100

> David Miller wrote:
> > But this time I'll just let you know up front that I
> > don't see much value in this patch.  It is not a clear
> > improvement to replace int's with bool's in my mind and
> > the other changes are just whitespace changes.
> >   
> Is it not an improvement to distinct booleans from actual values? Do you 
> use integers for ASCII characters too? It can also avoid some potential 
> bugs like the 'if (i == TRUE)'...
> What is wrong with 'size_t' (since it is unsigned, compared to (some) 
> 'int')?

When you say "int found;" is there any doubt in your mind that
this integer is going to hold a 1 or a 0 depending upon whether
we "found" something?

That's the problem I have with these kinds of patches, they do
not increase clarity, it's just pure mindless edits.

In new code, fine, use booleans if you want.

I would even accept that it helps to change to boolean for
arguments to functions that are global in scope.

But not for function local variables in cases like this.

  parent reply	other threads:[~2007-12-07  3:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-06 11:01 [Patch] net/xfrm/xfrm_policy.c: Some small improvements WANG Cong
2007-12-06 11:14 ` David Miller
2007-12-06 14:37   ` Richard Knutsson
2007-12-06 17:40     ` Herbert Xu
2007-12-07  3:22     ` David Miller [this message]
2007-12-07 15:41       ` Richard Knutsson

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=20071206.192249.193354742.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=akpm@osdl.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ricknu-0@student.ltu.se \
    --cc=xiyou.wangcong@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 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).