Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stable Serialization and Non-Exhaustiveness for EventKind Enum #109

Open
msk opened this issue Jun 30, 2023 · 0 comments
Open

Stable Serialization and Non-Exhaustiveness for EventKind Enum #109

msk opened this issue Jun 30, 2023 · 0 comments
Labels
good first issue Good for newcomers

Comments

@msk
Copy link
Contributor

msk commented Jun 30, 2023

This issue presents a potential enhancement to the EventKind enumeration within the review-database crate and proposes corresponding actions to address the identified needs.

At present, the serialized form of the EventKind enumeration is subject to potential alterations due to the automatic integer assignment performed by the Rust compiler during serialization. This could result in unintended changes to the serialized values of existing enumeration members when future modifications, such as the addition or deletion of members, occur.

To counter this potential problem, the proposal is to establish a stable serialized form for the EventKind enumeration. This can be accomplished by assigning a fixed serialized value to each enum member explicitly. By doing so, the serialized form of EventKind will remain stable, irrespective of future modifications to the enumeration. It's worth noting that the first enum member should be assigned 0 to maintain compatibility with the FromPrimitive trait. Here is a representative implementation:

#[derive(Serialize, Clone, Copy, Debug, Deserialize, Eq, FromPrimitive, PartialEq, ToPrimitive)]
#[repr(u32)]
#[allow(clippy::module_name_repetitions)]
pub enum EventKind {
    DnsCovertChannel = 0,
    HttpThreat = 1,
    RdpBruteForce = 2,
    //... Continue for other enum members
}

In addition, it is recommended that the EventKind enumeration is made non-exhaustive. By adding the #[non_exhaustive] attribute, future additions of new event kinds can be made without breaking existing code that matches on EventKind.

Here is the modified enum with the non-exhaustive attribute:

#[derive(Serialize, Clone, Copy, Debug, Deserialize, Eq, FromPrimitive, PartialEq, ToPrimitive)]
#[repr(u32)]
#[non_exhaustive]
#[allow(clippy::module_name_repetitions)]
pub enum EventKind {
    DnsCovertChannel = 0,
    HttpThreat = 1,
    RdpBruteForce = 2,
    //... Continue for other enum members
}

The successful implementation of these changes would contribute significantly to the robustness and adaptability of the review-database crate, ensuring it remains stable in light of any future changes.

@msk msk added the good first issue Good for newcomers label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant