All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Bill Wendling <morbo@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-toolchains@vger.kernel.org
Subject: Re: [PATCH v2 1/3] base: mark 'no_warn' as unused
Date: Tue, 27 Jul 2021 20:44:58 +0200	[thread overview]
Message-ID: <YQBUKrCWpM3uDp/Q@kroah.com> (raw)
In-Reply-To: <CAKwvOdkbgr5uPBUC815nrJ_-fHQD2KEYmZ0E8qZRBLL9uQ2WFw@mail.gmail.com>

On Tue, Jul 27, 2021 at 11:31:38AM -0700, Nick Desaulniers wrote:
> On Tue, Jul 27, 2021 at 10:59 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jul 27, 2021 at 10:39:49AM -0700, Nick Desaulniers wrote:
> > > If there are
> > > cases where it's ok to not check the return value, consider not using
> > > warn_unused_result on function declarations.
> >
> > Ok, so what do you do when you have a function like this where 99.9% of
> > the users need to check this?  Do I really need to write a wrapper
> > function just for it so that I can use it "safely" in the core code
> > instead?
> >
> > Something like:
> >
> > void do_safe_thing_and_ignore_the_world(...)
> > {
> >         __unused int error;
> >
> >         error = do_thing(...);
> > }
> >
> > Or something else to get the compiler to be quiet about error being set
> > and never used?  There HAS to be that option somewhere anyway as we need
> > it for other parts of the kernel where we do:
> >         write_bus(device, &value);
> >         value = read_bus(device);
> > and then we ignore value as it is not needed, but yet we still HAVE to
> > call read_bus() here, yet read_bus() is set as warn_unused_result()
> > because, well, it is a read function :)
> 
> Such wrappers are trivial with __attribute__((alias(""))):
> https://godbolt.org/z/j5afPbGcM
> 
> At least then it's very obvious if someone adds more call sites to
> such an alias. Then that calls for closer inspection in code review
> that yes, this is one of those 0.01% of cases.  Since they occur 0.01%
> of the time, I don't expect such aliases to occur too frequently.

That is just, well, horrible.  Seriously horrible.  Wow.

And that is the "documented" way to do this?  That feels like an abuse
of the already-horrible-why-do-they-do-that-for-variables use of the
alias attribute.

How badly are compiler people going to complain to me about this if
it's in this file?

I can take a patch for that, but I feel the comments involved will make
people, including myself when I have to look a the code again in 5
years, even more confused...

ick, I feel dirty...

greg k-h

  reply	other threads:[~2021-07-27 18:45 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  9:17 [PATCH 0/3] Fix clang -Wunused-but-set-variable warnings Bill Wendling
2021-07-14  9:17 ` [PATCH 1/3] base: remove unused variable 'no_warn' Bill Wendling
2021-07-14 12:15   ` kernel test robot
2021-07-14 12:15     ` kernel test robot
2021-07-14  9:17 ` [PATCH 2/3] bnx2x: remove unused variable 'cur_data_offset' Bill Wendling
2021-07-14  9:17 ` [PATCH 3/3] scsi: qla2xxx: remove unused variable 'status' Bill Wendling
2021-07-26 20:19 ` [PATCH v2 0/3] Fix clang -Wunused-but-set-variable warnings Bill Wendling
2021-07-26 20:19   ` [PATCH v2 1/3] base: mark 'no_warn' as unused Bill Wendling
2021-07-26 20:47     ` Nathan Chancellor
2021-07-26 21:01       ` Bill Wendling
2021-07-27  5:27       ` Greg Kroah-Hartman
2021-07-27  6:15         ` Bill Wendling
2021-07-27  6:41           ` Greg Kroah-Hartman
2021-07-27  7:08             ` Bill Wendling
2021-07-27  7:12               ` Greg Kroah-Hartman
2021-07-27  7:15                 ` Bill Wendling
2021-07-27 17:39             ` Nick Desaulniers
2021-07-27 17:42               ` Nick Desaulniers
2021-07-27 17:59               ` Greg Kroah-Hartman
2021-07-27 18:31                 ` Nick Desaulniers
2021-07-27 18:44                   ` Greg Kroah-Hartman [this message]
2021-07-27 19:02                     ` Nick Desaulniers
2021-07-27 19:23                 ` Bill Wendling
2021-07-27 20:13                 ` Segher Boessenkool
2021-07-27 20:22                   ` Bill Wendling
2021-07-27 20:24                     ` Bill Wendling
2021-07-27 18:32               ` Nathan Chancellor
2021-07-27 19:04                 ` Nick Desaulniers
2021-07-27 19:10                   ` Nathan Chancellor
2021-07-27 19:12                 ` Bill Wendling
2021-07-26 20:19   ` [PATCH v2 2/3] bnx2x: remove unused variable 'cur_data_offset' Bill Wendling
2021-07-26 20:36     ` Nathan Chancellor
2021-07-26 20:19   ` [PATCH v2 3/3] scsi: qla2xxx: remove unused variable 'status' Bill Wendling
2021-07-26 20:40     ` Nathan Chancellor
2021-07-27  3:14     ` Martin K. Petersen
2021-07-29  3:38   ` [PATCH v2 0/3] Fix clang -Wunused-but-set-variable warnings Martin K. Petersen

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=YQBUKrCWpM3uDp/Q@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=rafael@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.