1. Patchew
  2. linux
  3. View series

[PATCH] rust: device: Replace CString with CStr in property_present()

Viresh Kumar posted 1 patch 11 months ago 
rust/kernel/device.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] rust: device: Replace CString with CStr in property_present()
Posted by Viresh Kumar 11 months ago 
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>
---
Adapted with Rob's version a little.

 rust/kernel/device.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index e8c4cda5aacc..c775266ae164 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,7 +6,7 @@
 
 use crate::{
     bindings,
-    str::CString,
+    str::CStr,
     types::{ARef, Opaque},
 };
 use core::{fmt, ptr};
@@ -183,8 +183,8 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
     }
 
     /// Checks if property is present or not.
-    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 _) }
     }
 }
-- 
2.31.1.272.g89b43f80a514
Re: [PATCH] rust: device: Replace CString with CStr in property_present()
Posted by Alice Ryhl 11 months ago 
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.

Alice
Re: [PATCH] rust: device: Replace CString with CStr in property_present()
Posted by Gary Guo 11 months ago 
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
Re: [PATCH] rust: device: Replace CString with CStr in property_present()
Posted by Viresh Kumar 10 months, 4 weeks ago 
On 18-01-25, 00:44, Gary Guo wrote:
> 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()`?

Yes that works too..

-- 
viresh
Re: [PATCH] rust: device: Replace CString with CStr in property_present()
Posted by Viresh Kumar 11 months ago 
On 16-01-25, 09:25, Alice Ryhl 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.

Fixed as a separate commit. Thanks.

-- 
viresh
[PATCH] rust: device: Use as_char_ptr() to avoid explicit cast
Posted by Viresh Kumar 11 months ago 
Use as_char_ptr() to avoid explicit cast.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Rebased over:

https://lore.kernel.org/all/e97dcbe0418cc1053fb4bcfac65cc02a0afcdf78.1737005078.git.viresh.kumar@linaro.org/

 rust/kernel/device.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index c775266ae164..db2d9658ba47 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -185,7 +185,7 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
     /// Checks if property is present or not.
     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 _) }
+        unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_char_ptr()) }
     }
 }
 
-- 
2.31.1.272.g89b43f80a514
Re: [PATCH] rust: device: Use as_char_ptr() to avoid explicit cast
Posted by Alice Ryhl 11 months ago 
On Thu, Jan 16, 2025 at 9:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Use as_char_ptr() to avoid explicit cast.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.