Re: [RFC PATCH 07/25] tools/xenbindgen: Add support for structs in TOML specs

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Hi Alejandro,

Le 15/11/2024 à 12:51, Alejandro Vallejo a écrit :
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> ---
>   tools/rust/xenbindgen/src/c_lang.rs | 56 ++++++++++++++++++++++++-
>   tools/rust/xenbindgen/src/spec.rs   | 64 ++++++++++++++++++++++++++++-
>   2 files changed, 117 insertions(+), 3 deletions(-)
>
> diff --git a/tools/rust/xenbindgen/src/c_lang.rs 
> b/tools/rust/xenbindgen/src/c_lang.rs
> index f05e36bb362f..597e0ed41362 100644
> --- a/tools/rust/xenbindgen/src/c_lang.rs
> +++ b/tools/rust/xenbindgen/src/c_lang.rs
> @@ -17,9 +17,10 @@
>
>   use std::fmt::Write;
>
> -use crate::spec::OutFileDef;
> +use crate::spec::{OutFileDef, StructDef, Typ};
>
>   use convert_case::{Case, Casing};
> +use log::{debug, trace};
>
>   /// An abstract indentation level. 0 is no indentation, 1 is 
> [`INDENT_WIDTH`]
>   /// and so on.
> @@ -29,6 +30,39 @@ struct Indentation(usize);
>   /// Default width of each level of indentation
>   const INDENT_WIDTH: usize = 4;
>
> +/// Create a C-compatible struct field. Without the terminating semicolon.
> +fn structfield(typ: &Typ, name: &str) -> String {
> +    match typ {
> +        Typ::Ptr(x) => {
> +            let t: &Typ = x;
> +            format!(
> +                "XEN_GUEST_HANDLE_64({}) {name}",
> +                match t {
> +                    Typ::U8 => "uint8",
> +                    Typ::U16 => "uint16",
> +                    Typ::U32 => "uint32",
> +                    Typ::U64 => "uint64_aligned_t",
> +                    Typ::I8 => "int8",
> +                    Typ::I16 => "int16",
> +                    Typ::I32 => "int32",
> +                    Typ::I64 => "int64_aligned_t",
> +                    _ => panic!("foo {t:?}"),
> +                }
> +            )
> +        }
> +        Typ::Struct(x) => format!("struct {x} {name}"),
> +        Typ::Array(x, len) => format!("{}{name}[{len}]", structfield(x, "")),
> +        Typ::U8 => format!("uint8_t {name}"),
> +        Typ::U16 => format!("uint16_t {name}"),
> +        Typ::U32 => format!("uint32_t {name}"),
> +        Typ::U64 => format!("uint64_aligned_t {name}"),
> +        Typ::I8 => format!("int8_t {name}"),
> +        Typ::I16 => format!("int16_t {name}"),
> +        Typ::I32 => format!("int32_t {name}"),
> +        Typ::I64 => format!("int64_aligned_t {name}"),
> +    }
> +}
> +

I think _t are missing in the Ptr cases (we are currently generating
XEN_GUEST_HANDLE_64(uint8) which I don't think is valid).
Aside that, wouldn't it be better to have a separate function for
converting the type to its C representation ?

Something like

impl Typ { // or blanket trait
     fn c_repr(&self) -> String {
         match self {
             /* ... */
         }
     }
}

fn structfield(typ: &Typ, name: &str) -> String {
     format!("{} {name}", typ.c_repr());
}

We can also consider Typ::Struct or Typ::Array cases to call recursively
to c_repr on its inner type to get its representation.

That way, we can have XEN_GUEST_HANDLE_64(struct something).

Cheers

Teddy



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech