feat: add FromPropertyValue trait - #72
Conversation
| } | ||
| } | ||
|
|
||
| /// Returns the value of the standard `reg` property. |
There was a problem hiding this comment.
This is the part I dislike the most. Because of the differences in lifetimes between the fdt and model APIs, I don't think it's possible to avoid code duplication and maintain the ability to use the standard API while dropping the node objects it operates on - something tested by the newly added standard_node_outlives_wrapper test.
These kind of problems might suggest that maybe the GAT refactor from #29 wasn't the best idea ever - we traded the lifetime accuracy and code simplicity for better user ergonomics. Perhaps it's worth rolling back the change, and instead of trying to unify the read and write APIs, just rely on explicit to_read() method on the read APIs that would return a view of Node/Property.
If you have any thoughts on this @qwandor, I'm happy to hear them.
There was a problem hiding this comment.
I was a never a massive fan of the GAT refactor, so I'm happy for it to be rolled back if that helps avoid tying lifetimes together unneccessarily.
| pub fn enable_method(&self) -> Option<<<N as Node>::Property<'_> as Property>::StrList> { | ||
| Some(self.node.property("enable-method")?.as_str_list()) | ||
| #[must_use] | ||
| pub fn enable_method<'a>(&'a self) -> Option<crate::values::FdtStringListIterator<'a>> |
There was a problem hiding this comment.
Nit: Add a use statement for FdtStringListIterator.
| } | ||
| } | ||
|
|
||
| /// Returns the value of the standard `reg` property. |
There was a problem hiding this comment.
I was a never a massive fan of the GAT refactor, so I'm happy for it to be rolled back if that helps avoid tying lifetimes together unneccessarily.
|
|
||
| impl<'a, const N: usize> FromPropertyValue<'a> for [u32; N] { | ||
| fn from_property_value(value: &'a [u8]) -> Result<Self, PropertyError> { | ||
| let mut out = [0u32; N]; |
There was a problem hiding this comment.
If value.len() is not a multiple of 4 this silently drops the extra bytes at the end. Should it instead return an error?
| impl<'a> FromPropertyValue<'a> for Vec<u32> { | ||
| fn from_property_value(value: &'a [u8]) -> Result<Self, PropertyError> { | ||
| let mut out = Vec::with_capacity(value.len() / size_of::<u32>()); | ||
| for chunk in value.as_chunks::<{ size_of::<u32>() }>().0 { |
There was a problem hiding this comment.
Again, maybe this should return an error if the value isn't a multiple of 4 bytes long?
4b143d6 to
645686b
Compare
2 participants
No description provided.