Linux filesystem development
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Thorsten Blum <thorsten.blum@linux.dev>,
	Christian Brauner <brauner@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jeff Layton <jlayton@kernel.org>,
	Adrian Ratiu <adrian.ratiu@collabora.com>,
	xu xin <xu.xin16@zte.com.cn>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Felix Moessbauer <felix.moessbauer@siemens.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] proc: Use str_yes_no() helper in proc_pid_ksm_stat()
Date: Wed, 12 Feb 2025 11:04:51 -0500	[thread overview]
Message-ID: <20250212160451.GB106698@mit.edu> (raw)
In-Reply-To: <20250212123609.GP1977892@ZenIV>

On Wed, Feb 12, 2025 at 12:36:09PM +0000, Al Viro wrote:
> On Wed, Feb 12, 2025 at 01:11:08PM +0100, Thorsten Blum wrote:
> > /*
> >  * Here provide a series of helpers in the str_$TRUE_$FALSE format (you can
> >  * also expand some helpers as needed), where $TRUE and $FALSE are their
> >  * corresponding literal strings. These helpers can be used in the printing
> >  * and also in other places where constant strings are required. Using these
> >  * helpers offers the following benefits:
> >  *  1) Reducing the hardcoding of strings, which makes the code more elegant
> >  *     through these simple literal-meaning helpers.
> >  *  2) Unifying the output, which prevents the same string from being printed
> >  *     in various forms, such as enable/disable, enabled/disabled, en/dis.
> >  *  3) Deduping by the linker, which results in a smaller binary file.
> >  */
> 
> Printf modifiers would've covered all of that, though...
> 
> The thing is, <expr> ? "yes" : "no" is visually easier to distinguish than
> str_yes_no(<expr>), especially when expression itself is a function call, etc.
> So I'd question elegance, actually...

Yeah, personally I think str_yes_no() is a bit of a toss-up from an
elegance perspective.  It's a fewer number of characters, sure, but
it's a bit more cognitive load where the people reading the code need
to map str_yes_no() to semantic meaning.  For someone who is doing
wholesale conversions, they probably don't notice it, but for someone
who isn't dealing with it every day, it's One More Thing that they
have to track.

Using printf modifiers would probably actually save more bytes from a
"size of the binary" perspective, at the cost of forcing the code
reader to deal with some line-noise perl- or Rust-like construction
(e.g., if %pcy is str_yes_no, %pco is str_on_off, %pct is
str_true_false, etc. is the increased cognitive load worth it?)

Bottom line, I don't *hate* the string choices, but I really don't
think it's an obvious improvement.  Thorsten, this is why I hadn't
processed your ext4 patch yet.  I was super busy over the holidays,
and from a prioritization perspective, I didn't consider it high
priority.  Thinking about it now, to be honest, I'm feeling really
ambivilent about string_choices.h

Cheers,

					- Ted

      reply	other threads:[~2025-02-12 16:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-12 11:59 [PATCH] proc: Use str_yes_no() helper in proc_pid_ksm_stat() Thorsten Blum
2025-02-12 12:04 ` Al Viro
2025-02-12 12:11   ` Thorsten Blum
2025-02-12 12:36     ` Al Viro
2025-02-12 16:04       ` Theodore Ts'o [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=20250212160451.GB106698@mit.edu \
    --to=tytso@mit.edu \
    --cc=adrian.ratiu@collabora.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=felix.moessbauer@siemens.com \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=tglx@linutronix.de \
    --cc=thorsten.blum@linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xu.xin16@zte.com.cn \
    /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