linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: peterz@infradead.org, gregkh@linuxfoundation.org,
	 Dave Jiang <dave.jiang@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	 Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	 "Fabio M. De Francesco"
	<fabio.maria.de.francesco@linux.intel.com>,
	linux-kernel@vger.kernel.org,  linux-cxl@vger.kernel.org
Subject: Re: [PATCH 1/3] cleanup: Add cond_guard() to conditional guards
Date: Tue, 27 Feb 2024 12:49:34 -0800	[thread overview]
Message-ID: <CAHk-=win7bwWhPJ=iuW4h-sDTqbX6v9_LJnMaO3KxVfPSs81bQ@mail.gmail.com> (raw)
In-Reply-To: <170905253339.2268463.9376907713092612237.stgit@dwillia2-xfh.jf.intel.com>

On Tue, 27 Feb 2024 at 08:48, Dan Williams <dan.j.williams@intel.com> wrote:
>
>         cond_guard(mutex_intr, return -EINTR, &mutex);

Again, this is *not* helping make code readable and less likely to have bugs.

The macro has obvious deficiencies, like the "_fail" argument not
being surrounded by  "{ }" (the equivalent of parenthesizing an
expression argument), but even with that trivial fix the syntax is
just too ugly to live, and doesn't match normal C syntax.

And yes, we have other macros that don't have normal C syntax, and
they are ugly too (example: #define CHKINFO(ret) in
drivers/video/fbdev/hgafb.c), but we should have higher standards for
globally visible helpers, and we should have *MUCH* higher standards
for helpers that are supposed to be all about reducing mistakes.

Bad / odd syntax does not reduce mistakes.

If a sane 'guard' model doesn't work for some code, the answer is not
to make an insane guard model. The answer is to not use 'guard' in
code like that.

               Linus

  reply	other threads:[~2024-02-27 20:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-27 16:48 [PATCH 0/2] cleanup: A couple extensions for conditional resource management Dan Williams
2024-02-27 16:48 ` [PATCH 1/3] cleanup: Add cond_guard() to conditional guards Dan Williams
2024-02-27 20:49   ` Linus Torvalds [this message]
2024-02-27 16:48 ` [PATCH 2/3] cleanup: Introduce cond_no_free_ptr() Dan Williams
2024-02-27 20:40   ` Linus Torvalds
2024-02-27 16:49 ` [PATCH 3/3] cxl/region: Use cond_guard() in show_targetN() Dan Williams
2024-02-27 20:55   ` Linus Torvalds
2024-02-27 21:41     ` Dan Williams
2024-02-27 22:34       ` Linus Torvalds
2024-02-27 23:56         ` Dan Williams

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='CAHk-=win7bwWhPJ=iuW4h-sDTqbX6v9_LJnMaO3KxVfPSs81bQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=fabio.maria.de.francesco@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.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 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).