Re: [PATCH net-next RFC v3] net: hdlc_x25: Queue outgoing LAPB frames

From: Xie He <xie.he.0141@gmail.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Linux X25 <linux-x25@vger.kernel.org>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Martin Schiller <ms@dev.tdt.de>, Krzysztof Halasa <khc@pm.waw.pl>
Subject: Re: [PATCH net-next RFC v3] net: hdlc_x25: Queue outgoing LAPB frames
Date: Mon, 15 Feb 2021 23:30:25 -0800	[thread overview]
Message-ID: <CAJht_ENDJ-T_9-V04YUP-Qp+PnZnAcOeV+6eUXkTX4pmm5Vrag@mail.gmail.com> (raw)
In-Reply-To: <YCtgTBvR6TD8sPpe@unreal>

On Mon, Feb 15, 2021 at 10:04 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, Feb 15, 2021 at 11:08:02AM -0800, Xie He wrote:
> > On Mon, Feb 15, 2021 at 10:54 AM Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > > On Mon, Feb 15, 2021 at 09:23:32AM -0800, Xie He wrote:
> > > > On Mon, Feb 15, 2021 at 1:25 AM Leon Romanovsky <leon@kernel.org> wrote:
> > > > >
> > > > > > +     /* When transmitting data:
> > > > > > +      * first we'll remove a pseudo header of 1 byte,
> > > > > > +      * then the LAPB module will prepend an LAPB header of at most 3 bytes.
> > > > > > +      */
> > > > > > +     dev->needed_headroom = 3 - 1;
> > > > >
> > > > > 3 - 1 = 2
> > > > >
> > > > > Thanks
> > > >
> > > > Actually this is intentional. It makes the numbers more meaningful.
> > > >
> > > > The compiler should automatically generate the "2" so there would be
> > > > no runtime penalty.
> > >
> > > If you want it intentional, write it in the comment.
> > >
> > > /* When transmitting data, we will need extra 2 bytes headroom,
> > >  * which are 3 bytes of LAPB header minus one byte of pseudo header.
> > >  */
> > >  dev->needed_headroom = 2;
> >
> > I think this is unnecessary. The current comment already explains the
> > meaning of the "1" and the "3". There's no need for a reader of this
> > code to understand what a "2" is. That is the job of the compiler, not
> > the human reader.
>
> It is not related to compiler/human format. If you need to write "3 - 1"
> to make it easy for users, it means that your comment above is not
> full/correct/e.t.c.

My point is: there is no need for human programmers / code readers to
understand what this "2" is. There is no need to explain what this "2"
means in the comment. There is no need to write this "2" in the code.
There is no need for this "2" to appear anywhere. That is just an
intermediate result generated by the compiler. It is similar to
assembly or machine code. It is generated by the compiler. Human
programmers just don't care about this intermediate result.

My point could be more apparent if you consider a more complex
situation: "3 - 1 + 2 + 4 + 2". No human would want to see a
meaningless "10" in the code. We want to see the meaning of the
numbers, not a meaningless intermediate calculation result.