Re: [PATCH v8] rust: add new macro for common bitflag operations

[Andreas Hindborg <a.hindborg@kernel.org>]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>>
>>> We have seen a proliferation of mod_whatever::foo::Flags
>>> being defined with essentially the same implementation
>>> for BitAnd, BitOr, contains and etc.
>>>
>>> This macro aims to bring a solution for this,
>>> allowing to generate these methods for user-defined structs.
>>> With some use cases in KMS and upcoming GPU drivers.
>>>
>>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Suggested-by: Lyude Paul <lyude@redhat.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>>
>> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>>
>> I think it would be useful to add:
>>
>> impl Flags {
>>      unsafe from_raw(value: Repr) -> {
>>      ...
>>      }
>> }
>>
>> impl TryFrom<Repr> for Flag {
>>      // Succeed if `value` is a valid `Flag` enum bit pattern.
>> }
>>
>> impl TryFrom<Repr> for Flags {
>>      // Succeed if `value` is the logical OR of valid enum bit patterns.
>> }
>
> Also bitwise operations on the underlying type such as
>
>   impl BitOrAssign<Flag> for u32 {
>   ...
>   }
>
> Not sure if that is possible with the orphan rule, but it would be nice.
> I was trying to
>
>   lim.features |= request::Feature::Rotational;
>
> where `lim: bindings::queue_limits`.

Sorry for the spam. This works for me:

diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
index 0c50b8404fa79..e1143e6381d47 100644
--- a/rust/kernel/impl_flags.rs
+++ b/rust/kernel/impl_flags.rs
@@ -102,6 +102,25 @@ fn from(value: $flags) -> Self {
             }
         }
 
+        impl ::core::convert::From<$flag> for $ty {
+            #[inline]
+            fn from(value: $flag) -> Self {
+                value as _
+            }
+        }
+
+        impl ::core::convert::TryFrom<$ty> for $flags {
+            type Error = ::kernel::error::Error;
+
+            fn try_from(value: $ty) -> Result<Self, Self::Error> {
+                match value & !$flags::all_bits() == 0 {
+                    // SAFETY: We checked above that `value` is a valid bit pattern for `$flags`.
+                    true => Ok(unsafe {::core::mem::transmute(value)}),
+                    false => Err(::kernel::error::code::EINVAL),
+                }
+            }
+        }
+
         impl ::core::ops::BitOr for $flags {
             type Output = Self;
             #[inline]
@@ -225,6 +244,22 @@ fn bitxor(self, rhs: $flag) -> Self::Output {
             }
         }
 
+        impl ::core::ops::BitOr<$flag> for $ty {
+            type Output = Self;
+
+            #[inline]
+            fn bitor(self, rhs: $flag) -> Self::Output {
+                self | <$flag as Into<Self>>::into(rhs)
+            }
+        }
+
+        impl ::core::ops::BitOrAssign<$flag> for $ty {
+            #[inline]
+            fn bitor_assign(&mut self, rhs: $flag) {
+                *self = *self | <$flag as Into<Self>>::into(rhs)
+            }
+        }
+
         impl $flags {
             /// Returns an empty instance of `type` where no flags are set.
             #[inline]


Best regards,
Andreas Hindborg