Fix/arbitrary failing on feature flag#1128
Fix/arbitrary failing on feature flag#1128rajatdahal (razzat008) wants to merge 6 commits intoDevolutions:masterfrom
Conversation
This commit adds conditional attribute, which implements the `Arbitrary` trait when compiled with `--features arbitrary`; this is to ensure, sucessful checks/builds for future fuzzing implementation
|
Benoît Cortier (@CBenoit) |
Added conditional arbitrary for structs/enums that support it; files related to this commit most probably won't require manual impl of the Arbitrary trait. Due to inter-dependency, most of the struct/enums had to be configured
…uence The mannual impl for ChannelName follows the 8-bi null-terminated sequence; and the mannual impl for LicenseExchangeSequence generates arbitrary for all attributes except the license_cache
#cfg_attr for ChunkProcessor,StaticVirtualChannel and a constructor for the StaticChannelSet(as it has TypeId which is populated by the compiler)
|
I was not confident with some of the impls so I've added Mannual impls are done for:
|
|
The checks are passing for as of now: cargo check -p ironrdp-connector --features arbitrary
cargo check -p ironrdp-pdu --features arbitrary |
| pub domain: Option<String>, | ||
| pub hardware_id: [u32; 4], | ||
| pub license_cache: Arc<dyn LicenseCache>, | ||
| pub _phantom: std::marker::PhantomData<u8>, |
There was a problem hiding this comment.
question: Why did you add this PhantomData?
There was a problem hiding this comment.
I initially thought it was required for the dyn trait object to compile successfully, so I left it in there and forgot about it, even after the manual impl for LicenseExchangeSequence.
It compiles fine without it and isn't needed, my bad:))
| // MIGHT NEED CHANGE, currently it's not using arbitrary function | ||
| license_cache: Arc::new(NoopLicenseCache), |
There was a problem hiding this comment.
I think this is perfectly fine as we don’t provide any LicenseCache implementation as part of this crate. Implementers wishing to fuzz using their license cache should do so on their side.
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
I’m not entirely sure what the Arbitrary trait is meant to do in ironrdp-svc: we’re probably not going to deal with "arbitrary static virtual channels", so maybe a better way would be to manually implement the trait at some places more in ironrdp-connector 🤔
|
I understand, the structs in the ironrdp-svc crate were used in some other places which had to implement arbitrary, so without those checks wouldn't compile. |
2 participants
working on #1121