3 ms·

The caller takes ownership of the returned resources, i.e., will have the responsibility in calling `bindings::dma_free_attrs`. Hm, doesn't this violate RAII?

The caller takes ownership of the returned resources, i.e., will have the responsibility in calling `bindings::dma_free_attrs`. Hm, doesn't this violate RAII? Does this need to be public?

lmm 2y ago
> doesn't this violate RAII? It's C, you don't get any RAII.
- meltyness 2y ago
  Not the Rust abstraction. If mutation is needed, you'd write closures into the interface, or provider borrows. It's definitely weird to barf out ownership of half of the fields. E: I guess it sort of depends whether the handle is Copy semantics or not. E2: I guess they're correctly leaking the resource, but I question whether they should, protections like that are kind of the whole point of using Rust to begin with.
- pjmlp 2y ago
  No, but you get defer via compiler extensions, and maybe in C2y as standard language feature.
  - flohofwoe 2y ago
    I seem to remember that the Linux kernel already uses __attribute__((cleanup) (which is basically GCC's non-standard defer).
    - pjmlp 2y ago
      I think the Linux kernel uses most GCC extensions, and they also disable strict aliasing. :)
Dagonfly 2y ago
This is similar to `Box::into_raw` in Rust `std`. It destructs the RAII struct into a raw pointer. After that it's pretty much expected that you are responsible for cleanup.
- meltyness 2y ago
  Seems like the preferred interface would be a purely functional block that provides the access. Obviously forcing callers to free means adding lints and docs. Returning borrows leaves the likelihood of entering into explicit lifetimes hell and such in calling code. A functional interface like (paraphrasing) // pub fn apply_op(&mut self, FnMut(&mut T, &bindings::dma_addr_t... )) Keeps pandora inside the box.
  - Dagonfly 2y ago
    As most of the kernel is in C, they probably need a raw pointer quite regularly to pass it into existing C APIs. Just like `Box::into_raw`, this method shouldn't be used in pure Rust code. It's only available for C FFI.
    - meltyness 2y ago
      Right but, this isn't merely FFI code, those are via Bindgen, this is the abstraction intended to be provided to all DMA driver authors as the API, as I understand it.
Tobu 2y ago
The method is called `CoherentAllocation::into_parts`. And it takes self by value, which means the struct is consumed (this is implied by the into_ prefix). Of course you would either reconstruct the allocation or free the allocation yourself after this. See Vec::into_raw_parts or Box::into_raw for an stdlib analogy.

The caller takes ownership of the returned resources, i.e., will have the responsibility in calling `bindings::dma_free_attrs`. Hm, doesn't this violate RAII? Does this need to be public?

lmm 2y ago

> doesn't this violate RAII? It's C, you don't get any RAII.

meltyness 2y ago
Not the Rust abstraction. If mutation is needed, you'd write closures into the interface, or provider borrows. It's definitely weird to barf out ownership of half of the fields. E: I guess it sort of depends whether the handle is Copy semantics or not. E2: I guess they're correctly leaking the resource, but I question whether they should, protections like that are kind of the whole point of using Rust to begin with.
pjmlp 2y ago
No, but you get defer via compiler extensions, and maybe in C2y as standard language feature.
- flohofwoe 2y ago
  I seem to remember that the Linux kernel already uses __attribute__((cleanup) (which is basically GCC's non-standard defer).
  - pjmlp 2y ago
    I think the Linux kernel uses most GCC extensions, and they also disable strict aliasing. :)

Dagonfly 2y ago

This is similar to `Box::into_raw` in Rust `std`. It destructs the RAII struct into a raw pointer. After that it's pretty much expected that you are responsible for cleanup.

meltyness 2y ago
Seems like the preferred interface would be a purely functional block that provides the access. Obviously forcing callers to free means adding lints and docs. Returning borrows leaves the likelihood of entering into explicit lifetimes hell and such in calling code. A functional interface like (paraphrasing) // pub fn apply_op(&mut self, FnMut(&mut T, &bindings::dma_addr_t... )) Keeps pandora inside the box.
- Dagonfly 2y ago
  As most of the kernel is in C, they probably need a raw pointer quite regularly to pass it into existing C APIs. Just like `Box::into_raw`, this method shouldn't be used in pure Rust code. It's only available for C FFI.
  - meltyness 2y ago
    Right but, this isn't merely FFI code, those are via Bindgen, this is the abstraction intended to be provided to all DMA driver authors as the API, as I understand it.

Tobu 2y ago

The method is called `CoherentAllocation::into_parts`. And it takes self by value, which means the struct is consumed (this is implied by the into_ prefix). Of course you would either reconstruct the allocation or free the allocation yourself after this. See Vec::into_raw_parts or Box::into_raw for an stdlib analogy.