mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Carlos Llamas" <cmllamas@google.com>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Benno Lossin" <lossin@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Lyude Paul" <lyude@redhat.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Waiman Long" <longman@redhat.com>,
	"Will Deacon" <will@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v5 4/5] rust_binder: consolidate transaction failure prints
Date: Thu, 1 Oct 2026 14:32:41 +0200	[thread overview]
Message-ID: <2026100130-perjurer-frisk-4af2@gregkh> (raw)
In-Reply-To: <CANiq72kb=X1igu+pK=KmAxqDncGYF9zpxrihSuSD3WaCUm336Q@mail.gmail.com>

On Tue, Sep 01, 2026 at 06:15:52PM +0200, Miguel Ojeda wrote:
> On Mon, Aug 3, 2026 at 9:30 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> > index a56ba6309594..380cd3f7276b 100644
> > --- a/rust/kernel/error.rs
> > +++ b/rust/kernel/error.rs
> > @@ -135,7 +135,7 @@ pub fn from_errno(errno: crate::ffi::c_int) -> Error {
> >      /// Creates an [`Error`] from a kernel error code.
> >      ///
> >      /// Returns [`None`] if `errno` is out-of-range.
> > -    const fn try_from_errno(errno: crate::ffi::c_int) -> Option<Error> {
> > +    pub const fn try_from_errno(errno: crate::ffi::c_int) -> Option<Error> {
> >          if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 {
> >              return None;
> >          }
> 
> Generally speaking, one should know from the context whether an
> integer is supposed to be an error or not, and thus it is rare to need
> this function instead of the public one (this one is private, and the
> two callers are here, not elsewhere in `kernel`).
> 
> So I wondered if Binder needs this -- I noticed the change when doing
> my usual go-through-the-ML exercise and asked Alice about it, since it
> seemed to me like Binder could perhaps avoid using the fallible
> operation (and maybe even define an `enum` for `BinderError` instead
> of a `struct` to be more precise about when a `source` is needed).
> 
> Alice told me that the `Option` in `BinderError` is just meant for the
> zero case, i.e. the raw integer there should not be a random value.
> Thus, since the `if` already covers the zero case, it does look like
> this could use the infallible operation since we do know statically it
> should be an error (modulo a bug).
> 
> So it sounds like the change can indeed be avoided, which should also
> improve the code.
> 
> In any case, if we keep it, then the `error.rs` change should be
> mentioned in the commit message.
> 
> By the way, I am still happy to take the first three patches unless
> Binder is picking this up.

I'll just take them all now, thanks.

greg k-h

  reply	other threads:[~2026-10-01 12:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-03  7:29 [PATCH v5 0/5] Rate limited printing for Rust Alice Ryhl
2026-08-03  7:29 ` [PATCH v5 1/5] rust: sync: move lockdep types to rust/kernel/sync/lockdep.rs Alice Ryhl
2026-08-03  7:29 ` [PATCH v5 2/5] rust: sync: add const constructor for raw_spinlock_t Alice Ryhl
2026-08-03  7:29 ` [PATCH v5 3/5] rust: add pr_*_ratelimit! macros for printing Alice Ryhl
2026-08-03  7:29 ` [PATCH v5 4/5] rust_binder: consolidate transaction failure prints Alice Ryhl
2026-09-01 16:15   ` Miguel Ojeda
2026-10-01 12:32     ` Greg Kroah-Hartman [this message]
2026-10-01 12:33       ` Greg Kroah-Hartman
2026-08-03  7:29 ` [PATCH v5 5/5] rust_binder: use pr_*_ratelimited! for printing Alice Ryhl

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=2026100130-perjurer-frisk-4af2@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=cmllamas@google.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=mingo@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=will@kernel.org \
    --cc=work@onurozkan.dev \
    /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

all inboxes | Powered by JetHome®