Re: [PATCH v2 16/19] gendwarfksyms: Add support for reserved structure fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20.08.24 22:03, Matthew Maurer wrote:
>>> The way `KAbiReserved` is implemented is via a `union` (maybe a bit
>>> ironic, considering what I said in my other replies, but in this case,
>>> we would provide a safe abstraction over this `union`, thus avoiding
>>> exposing users of this type to `unsafe`):
>>>
>>>     #[repr(C)]
>>>     pub union KAbiReserved<T, R> {
>>>         value: T,
>>>         _reserved: R,
>>>     }
>>
>> I like this approach even better, assuming any remaining issues with
>> ownership etc. can be sorted out. This would also look identical to
>> the C version in DWARF if you rename _reserved in the union to
>> __kabi_reserved. Of course, we can always change gendwarfksyms to
>> support a different scheme for Rust code if a better solution comes
>> along later.

Yeah sure, that should also then work directly with this patch, right?

>> Sami
> 
> Agreement here - this seems like a good approach to representing
> reserved in Rust code. A few minor adjustments we discussed off-list
> which aren't required for gendwarfksyms to know about:
> 1. Types being added to reserved fields have to be `Copy`, e.g. they
> must be `!Drop`.
> 2. Types being added to reserved fields must be legal to be
> represented by all zeroes.
> 3. Reserved fields need to be initialized to zero before having their
> union set to the provided value when constructing them.
> 4. It may be helpful to have delegating trait implementations to avoid
> the couple places where autoderef won't handle the conversion.
> 
> While I think this is the right solution, esp. since it can share a
> representation with C, I wanted to call out one minor shortfall - a
> reserved field can only be replaced by one type. We could still
> indicate a replacement by two fields the same as in C, by using a
> tuple which will look like an anonymous struct. The limitation will be
> that if two or more new fields were introduced, we'd need to edit the
> patches accessing them to do foo.x.y and foo.x.z for their accesses
> instead of simply foo.y and foo.z - the autoref trick only works for a
> single type.

We will have to see how often multiple fields are added to a struct. If
they are infrequent and it's fine for those patches to then touch the
field accesses, then I think we can just stick with this approach.
If there are problems with that, we can also try the following:
all fields of kABI structs must be private and must only be accessed
through setters/getters. We can then modify the body the setters/getters
to handle the additional indirection.

If that also sounds too much work compared to the C side, then we can
get in touch with the Rust language folks and see if they can provide us
with a better solution.

---
Cheers,
Benno






[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux