search

librasn / rasn

A Safe #[no_std] ASN.1 Codec Framework
Other
183 stars 43 forks source link

refactor(macros): Treat Unit Structs as ASN.1 NULL #227

Closed 6d7a closed 4 months ago

6d7a commented 4 months ago

Closes #225

Replaces the empty struct of rasn_ldap::UnbindRequest with a null delegate. @michaldrabina we use rust's unit value () as an equivalent for ASN.1's NULL value. That way, you can simply use the delegate annotation. I couldn't see that it breaks any existing code, but let me know in case. Thanks for the issue!

XAMPPRocky commented 4 months ago

Thank you for your PR! Though I wonder if we should instead generate null from these kinds of structs instead of a delegate, I don't see where the empty sequences for unit structs are useful.

6d7a commented 4 months ago

From the top of my head 😄 , ISO8571-FTAM contains some empty sequences. I guess it's better to be explicit with the null delegate.

XAMPPRocky commented 4 months ago

Sure, but I guess I would consider the syntax for empty sequences to be the following, where it's defined to have fields, it's just the fields are empty.

struct FBeginGroupResponse {}

Where as in Rust the syntax we're using currently is called a "unit struct" meaning that semantically it is identical to (), but it is a better description of a single value.

struct UnbindRequest;
6d7a commented 4 months ago

I see, makes sense. Should we move that refactor to a separate issue? By the way, do you think we can go ahead with PR #159?

XAMPPRocky commented 4 months ago

Should we move that refactor to a separate issue?

I'm not sure what you mean, fixing that should resolve the same issue right?

6d7a commented 4 months ago

I adapted the macros so that unit structs are now interpreted as ASN.1 NULL.

XAMPPRocky commented 4 months ago

Thank you for your PR!