From: Gary Guo <gary@garyguo.net>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Viresh Kumar" <viresh.kumar@linaro.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"Danilo Krummrich" <dakr@kernel.org>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Rob Herring" <robh@kernel.org>,
"Dirk Behme" <dirk.behme@de.bosch.com>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rust: device: Replace CString with CStr in property_present()
Date: Sat, 18 Jan 2025 00:44:43 +0000 [thread overview]
Message-ID: <20250118004443.51cc6302.gary@garyguo.net> (raw)
In-Reply-To: <CAH5fLgjsuf+smPg5734mGZnZRFS9yDrFQnREhWwSVqoxP1AuBg@mail.gmail.com>
On Thu, 16 Jan 2025 09:25:17 +0100
Alice Ryhl <aliceryhl@google.com> wrote:
> On Thu, Jan 16, 2025 at 6:26 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > The property_present() method expects a &CString currently and will work
> > only with heap allocated C strings.
> >
> > In order to make it work with compile-time string constants too, change
> > the argument type to &CStr.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> One nit below, but either way:
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> > - pub fn property_present(&self, name: &CString) -> bool {
> > - // SAFETY: By the invariant of `CString`, `name` is null-terminated.
> > + pub fn property_present(&self, name: &CStr) -> bool {
> > + // SAFETY: By the invariant of `CStr`, `name` is null-terminated.
> > unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_ptr() as *const _) }
>
> If you use `name.as_char_ptr()` then you don't need the cast.
Isn't the cast not needed anyway with `as_ptr()`?
c_char is unconditionally u8 now so they're now the same.
Best,
Gary
>
> Alice
next prev parent reply other threads:[~2025-01-18 0:44 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-16 5:26 [PATCH] rust: device: Replace CString with CStr in property_present() Viresh Kumar
2025-01-16 8:25 ` Alice Ryhl
2025-01-16 8:35 ` Viresh Kumar
2025-01-18 0:44 ` Gary Guo [this message]
2025-01-20 5:05 ` Viresh Kumar
2025-01-16 8:34 ` [PATCH] rust: device: Use as_char_ptr() to avoid explicit cast Viresh Kumar
2025-01-16 9:01 ` 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=20250118004443.51cc6302.gary@garyguo.net \
--to=gary@garyguo.net \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=dirk.behme@de.bosch.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
--cc=viresh.kumar@linaro.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