From: Joe Perches <joe@perches.com>
To: "Luciano Coelho" <luciano.coelho@intel.com>,
"Christoph Böhmwalder" <christoph@boehmwalder.at>,
johannes.berg@intel.com, emmanuel.grumbach@intel.com,
kvalo@codeaurora.org
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] wireless: iwlwifi: use bool instead of int
Date: Wed, 04 Oct 2017 10:55:22 -0700 [thread overview]
Message-ID: <1507139722.4434.12.camel@perches.com> (raw)
In-Reply-To: <1507135159.908.96.camel@intel.com>
On Wed, 2017-10-04 at 19:39 +0300, Luciano Coelho wrote:
> On Wed, 2017-10-04 at 09:26 -0700, Joe Perches wrote:
[]
> > This might be more intelligble as separate tests
> >
> > static bool is_valid_channel(u16 ch_id)
> > {
> > if (ch_id <= 14)
> > return true;
> >
> > if ((ch_id % 4 == 0) &&
> > ((ch_id >= 36 && ch_id <= 64) ||
> > (ch_id >= 100 && ch_id <= 140)))
> > return true;
> >
> > if ((ch_id % 4 == 1) &&
> > (chid >= 145 && ch_id <= 165))
> > return true;
> >
> > return false;
> > }
> >
> > The compiler should produce the same object code.
>
> Yeah, it may be a bit easier to read, but I don't want to start getting
> "fixes" to working and reasonable code. There's nothing wrong with the
> existing function (except maybe for the int vs. boolean) so let's not
> change it.
>
> A good time to change this would be the next time someone adds yet
> another range of valid channels here. ;)
<shrug> Your choice.
I like code I can read and understand at a glance.
At case somebody needs to add channels, likely nobody
would do the change suggested but would just add
another test to the already odd looking block.
And constants should be on the right side of the tests.
next prev parent reply other threads:[~2017-10-04 17:55 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-04 15:56 [PATCH 0/3] iwlwifi: cosmetic fixes Christoph Böhmwalder
2017-10-04 15:56 ` [PATCH 1/3] wireless: iwlwifi: use bool instead of int Christoph Böhmwalder
2017-10-04 16:15 ` Luciano Coelho
2017-10-04 16:26 ` Joe Perches
2017-10-04 16:39 ` Luciano Coelho
2017-10-04 17:55 ` Joe Perches [this message]
2017-10-04 19:18 ` Luciano Coelho
2017-10-04 15:56 ` [PATCH 2/3] wireless: iwlwifi: function definition cosmetic fix Christoph Böhmwalder
2017-10-04 16:13 ` Luciano Coelho
2017-10-04 15:57 ` [PATCH 3/3] wireless: iwlwifi: wrap macro into braces Christoph Böhmwalder
2017-10-04 16:08 ` Johannes Berg
2017-10-04 16:14 ` Joe Perches
2017-10-04 16:21 ` [PATCH 0/3] iwlwifi: cosmetic fixes Luciano Coelho
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=1507139722.4434.12.camel@perches.com \
--to=joe@perches.com \
--cc=christoph@boehmwalder.at \
--cc=emmanuel.grumbach@intel.com \
--cc=johannes.berg@intel.com \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=luciano.coelho@intel.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.