Re: [PATCH v8 2/2] rust: add dma coherent allocator abstraction.

Daniel Sedlak
2025-01-08T18:08:32Z

On 1/8/25 1:27 PM, Abdiel Janulgue wrote:
> +/// Possible attributes associated with a DMA mapping.
> +///
> +/// They can be combined with the operators `|`, `&`, and `!`.
> +///
> +/// Values can be used from the [`attrs`] module.
> +#[derive(Clone, Copy, PartialEq)]
> +pub struct Attrs(pub u32);

Shouldn't this be non-pub? It seems unfortunate to leak C representation
to the Rust world and allow its construction without going through
constructor method. Also #[repr(transparent)] may be well suited? [1]

Link: https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent
[1]
> +
> +impl Attrs {
> + /// Get the raw representation of this attribute.
> + pub(crate) fn as_raw(self) -> usize {
> + self.0.try_into().unwrap()
> + }

Shouldn't be the return type u32 instead of usize? Or at least to my
understanding the raw means C representation which should be the type
that bindgen generates.
> +
> + /// Check whether `flags` is contained in `self`.
> + pub fn contains(self, flags: Attrs) -> bool {
> + (self & flags) == flags
> + }
> +}
Daniel