netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Asbjørn Sloth Tønnesen" <ast@fiberby.net>
Cc: "David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"Arkadiusz Kubalewski" <arkadiusz.kubalewski@intel.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Daniel Zahka" <daniel.zahka@gmail.com>,
	"Donald Hunter" <donald.hunter@gmail.com>,
	"Jacob Keller" <jacob.e.keller@intel.com>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	"Jiri Pirko" <jiri@resnulli.us>,
	"Joe Damato" <jdamato@fastly.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Simon Horman" <horms@kernel.org>,
	"Stanislav Fomichev" <sdf@fomichev.me>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Vadim Fedorenko" <vadim.fedorenko@linux.dev>,
	"Willem de Bruijn" <willemb@google.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code
Date: Tue, 14 Oct 2025 12:24:41 -0700	[thread overview]
Message-ID: <20251014122441.27e2d267@kernel.org> (raw)
In-Reply-To: <d3f1427f-e8bc-4ab0-bf15-171b701325b9@fiberby.net>

On Tue, 14 Oct 2025 16:49:22 +0000 Asbjørn Sloth Tønnesen wrote:
> On 10/14/25 12:53 AM, Jakub Kicinski wrote:
> > On Mon, 13 Oct 2025 16:49:58 +0000 Asbjørn Sloth Tønnesen wrote:  
> >> Instead of pre-computing the flag values within the code generator,
> >> then move the bitshift operation into the generated code.
> >>
> >> This IMHO makes the generated code read more like handwritten code.  
> > 
> > I like it the way it is. The values are irrelevant.  
> 
> Bit-shifting seams like the preferred way across the uAPI headers.
> 
> Would you be open to hexadecimal notation, if not bit-shifting?
> 
> Currently NLA_POLICY_MASK() is generated with a hexadecimal mask, and
> with these patches, if render-max is not set. If using literal values
> then we should properly consistently generate them as either decimal
> or hexadecimal. I prefer hexadecimal over decimal.

Hm, hex could do. For the bit/1 << x i really don't like that the values
are not aligned to columns, so they visually mix in with the names. 
But aligning them would be more LoC than it's worth.

hex could be a reasonable compromise, but I make no promise that I will
like it once I see the result :)

> > And returning a string from user_value() is quite ugly.  
> It only returns a string, when as_c is set, I am happy to duplicate
> some code instead, and add a dedicated method always returning a string,
> but can we please agree on the generated output, before implementation?

nlspec.py was supposed to be a library that abstracts away things like
default values not being present, and simplifies indexing. So having a
"give me a format for C as result" arg is not great for layering.
That kind of logic belongs in the caller. 

Regarding LoC - great code is concise, but that doesn't mean that
making code shorter always makes it better.

  reply	other threads:[~2025-10-14 19:24 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13 16:49 [PATCH net-next 0/6] tools: ynl-gen: generate flags better Asbjørn Sloth Tønnesen
2025-10-13 16:49 ` [PATCH net-next 1/6] tools: ynl-gen: bitshift the flag values in the generated code Asbjørn Sloth Tønnesen
2025-10-13 23:07   ` Jacob Keller
2025-10-14 16:27     ` Asbjørn Sloth Tønnesen
2025-10-14  0:53   ` Jakub Kicinski
2025-10-14 16:49     ` Asbjørn Sloth Tønnesen
2025-10-14 19:24       ` Jakub Kicinski [this message]
2025-10-13 16:49 ` [PATCH net-next 2/6] tools: ynl-gen: refactor render-max enum generation Asbjørn Sloth Tønnesen
2025-10-14  0:58   ` Jakub Kicinski
2025-10-14 17:04     ` Asbjørn Sloth Tønnesen
2025-10-14 19:26       ` Jakub Kicinski
2025-10-13 16:50 ` [PATCH net-next 3/6] tools: ynl-gen: use uapi mask definition in NLA_POLICY_MASK Asbjørn Sloth Tønnesen
2025-10-14  0:59   ` Jakub Kicinski
2025-10-14 17:29     ` Asbjørn Sloth Tønnesen
2025-10-14 19:32       ` Jakub Kicinski
2025-10-13 16:50 ` [PATCH net-next 4/6] tools: ynl-gen: add generic p_wrap() helper Asbjørn Sloth Tønnesen
2025-10-13 16:50 ` [PATCH net-next 5/6] tools: ynl-gen: construct bitflag masks in generated headers Asbjørn Sloth Tønnesen
2025-10-13 16:50 ` [PATCH net-next 6/6] tools: ynl-gen: allow custom naming of render-max definitions Asbjørn Sloth Tønnesen
2025-10-13 23:10 ` [PATCH net-next 0/6] tools: ynl-gen: generate flags better Jacob Keller

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=20251014122441.27e2d267@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=ast@fiberby.net \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=daniel.zahka@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=horms@kernel.org \
    --cc=jacob.e.keller@intel.com \
    --cc=jdamato@fastly.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=toke@redhat.com \
    --cc=vadim.fedorenko@linux.dev \
    --cc=willemb@google.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).