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

Semantic serialization requires Clone implementation #1793

Closed
lauralt opened this issue Apr 15, 2020 · 3 comments
Closed

Semantic serialization requires Clone implementation #1793

lauralt opened this issue Apr 15, 2020 · 3 comments

Comments

@lauralt
Copy link

lauralt commented Apr 15, 2020

If a struct field has a semantic serialization function defined, the whole structure must implement Clone. This is inefficient and can also cause trouble when having also a FamStructWrapper<T> field defined in that structure.
Calling clone() on a FamStructWrapper<T> instance will clone only the number of entries and the flexible array members. For example, if we have the following structure:

pub struct kvm_cpuid {
    pub nent: __u32,
    pub padding: __u32,
    pub entries: __IncompleteArrayField<kvm_cpuid_entry>,
}

wrapped in a FamStructWrapper and we call clone() on it, only nent and entries fields will be kept (padding field will take the default u32 value).

So, if we want to serialize the following structure:

#[derive(Versionize, Clone)]
pub struct S {
    some_kvm_cpuid: FamStructWrapper<kvm_cpuid>,
    #[version(start = 2, ser_fn="ser_u16")]
    some_u16: u16,
} 

clone() will be called before we actually serialize the fields and because of this, we will serialize the default value for padding field, not the value that we pass when we instantiate S.

@andreeaflorescu
Copy link
Member

We should definitely check if we can relax the restrictions on the Versionize interface so that clone is not needed.

At the same time, having FamStruct clone implemented in such a way that it is only "cloning" the number of entries and the actual entries doesn't seem correct to me. Can you also open an issue in vmm-sys-util so that we can fix it (or remove the autogenerated clone)?

@lauralt
Copy link
Author

lauralt commented Apr 15, 2020

Issue here: rust-vmm/vmm-sys-util#85.

@AlexandruCihodaru
Copy link
Contributor

This is not a current pain point/blocker for anything and if it becomes one we will reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants