Re: [PATCH v3] xen/public: fix flexible array definitions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.12.2023 12:20, Juergen Gross wrote:
> Flexible arrays in public headers can be problematic with some
> compilers.
> 
> With XEN_FLEX_ARRAY_DIM there is a mechanism available to deal with
> this issue, but care must be taken to not change the affected structs
> in an incompatible way.
> 
> So bump __XEN_LATEST_INTERFACE_VERSION__ and introduce a new macro
> XENPV_FLEX_ARRAY_DIM which will be XENPV_FLEX_ARRAY_DIM with the
> interface version being new enough and "1" (the value used today in
> the affected headers) when the interface version is an old one.
> 
> Replace the arr[1] instances (this includes the ones seen to be
> problematic in recent Linux kernels [1]) with arr[XENPV_FLEX_ARRAY_DIM]
> in order to avoid compilation errors.
> 
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=217693
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/xen/include/public/io/ring.h
> +++ b/xen/include/public/io/ring.h
> @@ -25,8 +25,16 @@
>   * and grant_table.h from the Xen public headers.
>   */
>  
> +#include "../xen.h"
>  #include "../xen-compat.h"

Just to mention it: While perhaps good practice, I'm not convinced this
extra #include is actually needed here.

> +/* Some PV I/O interfaces need a compatibility variant. */
> +#if __XEN_INTERFACE_VERSION__ < 0x00041300
> +#define XENPV_FLEX_ARRAY_DIM  1 /* variable size */
> +#else
> +#define XENPV_FLEX_ARRAY_DIM  XEN_FLEX_ARRAY_DIM
> +#endif
> +
>  #if __XEN_INTERFACE_VERSION__ < 0x00030208
>  #define xen_mb()  mb()
>  #define xen_rmb() rmb()
> @@ -110,7 +118,7 @@ struct __name##_sring {                                   
>               \
>          uint8_t pvt_pad[4];                                             \
>      } pvt;                                                              \
>      uint8_t __pad[44];                                                  \
> -    union __name##_sring_entry ring[1]; /* variable-length */           \
> +    union __name##_sring_entry ring[XENPV_FLEX_ARRAY_DIM];              \
>  };                                                                      \
>                                                                          \
>  /* "Front" end's private variables */                                   \
> @@ -479,7 +487,7 @@ struct name##_data_intf {                                 
>                     \
>      uint8_t pad2[56];                                                        
>  \
>                                                                               
>  \
>      RING_IDX ring_order;                                                     
>  \
> -    grant_ref_t ref[];                                                       
>  \
> +    grant_ref_t ref[XEN_FLEX_ARRAY_DIM];                                     
>  \
>  };                                                                           
>  \
>  DEFINE_XEN_FLEX_RING(name)
>