From: binyi <dantengknight@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Joe Perches <joe@perches.com>,
Manish Chopra <manishc@marvell.com>,
GR-Linux-NIC-Dev@marvell.com, Coiby Xu <coiby.xu@gmail.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
netdev@vger.kernel.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] staging: qlge: Fix indentation issue under long for loop
Date: Sun, 17 Jul 2022 19:12:41 -0700 [thread overview]
Message-ID: <20220718021241.GA8270@cloud-MacBookPro> (raw)
In-Reply-To: <20220715092847.GU2316@kadam>
On Fri, Jul 15, 2022 at 12:28:47PM +0300, Dan Carpenter wrote:
> On Thu, Jul 14, 2022 at 11:04:57PM -0700, binyi wrote:
> > On Tue, Jul 12, 2022 at 07:14:55AM -0700, Joe Perches wrote:
> > > On Tue, 2022-07-12 at 16:46 +0300, Dan Carpenter wrote:
> > > > > @@ -3007,10 +3007,12 @@ static int qlge_start_rx_ring(struct qlge_adapter *qdev, struct rx_ring *rx_ring
> > > > > tmp = (u64)rx_ring->lbq.base_dma;
> > > > > base_indirect_ptr = rx_ring->lbq.base_indirect;
> > > > >
> > > > > - for (page_entries = 0; page_entries <
> > > > > - MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++)
> > > > > - base_indirect_ptr[page_entries] =
> > > > > - cpu_to_le64(tmp + (page_entries * DB_PAGE_SIZE));
> > > > > + for (page_entries = 0;
> > > > > + page_entries < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN);
> > > > > + page_entries++) {
> > > > > + base_indirect_ptr[page_entries] = cpu_to_le64(tmp);
> > > > > + tmp += DB_PAGE_SIZE;
> > > >
> > > > I've previously said that using "int i;" is clearer here. You would
> > > > kind of expect "page_entries" to be the number of entries, so it's kind
> > > > of misleading. In other words, it's not just harmless wordiness and
> > > > needless exposition, it's actively bad.
> > >
> > > Likely true.
> > >
> >
> > I agree it could be misleading. But if "page_entries" is in the for loop I
> > would assume it's some kind of index variable, and still it provides some
> > information (page entry) for the index, probably page_entry_idx could be
> > better name but still makes the for loop a very long one. I guess I would
> > leave it be.
>
> It does not "provide some information". That's what I was trying to
> explain. It's the opposite of information. Information is good. No
> information is neutral. But anti-information is bad.
I see. Indeed, being neutral is better than being bad.
> Like there are so many times in life where you listen to someone and you
> think you are learning something but you end up stupider than before.
> They have done studies on TV news where it can make you less informed
> than people who don't watch it. And other studies say if you stop
> watching TV news for even a month then your brain starts to heal.
>
> I don't really care about one line of code, but what I'm trying to say
> is learn to recognize anti-information and delete it. Comments for
> example are often useless.
It makes more sense to me now. Thanks for explaining!
Best,
Binyi
prev parent reply other threads:[~2022-07-18 2:12 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-10 21:04 [PATCH v3] staging: qlge: Fix indentation issue under long for loop Binyi Han
2022-07-11 8:05 ` Greg Kroah-Hartman
2022-07-11 20:55 ` Joe Perches
2022-07-12 9:44 ` Greg Kroah-Hartman
2022-07-13 8:14 ` binyi
2022-07-12 13:46 ` Dan Carpenter
2022-07-12 14:14 ` Joe Perches
2022-07-15 6:04 ` binyi
2022-07-15 9:28 ` Dan Carpenter
2022-07-18 2:12 ` binyi [this message]
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=20220718021241.GA8270@cloud-MacBookPro \
--to=dantengknight@gmail.com \
--cc=GR-Linux-NIC-Dev@marvell.com \
--cc=coiby.xu@gmail.com \
--cc=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=manishc@marvell.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.