[Mesa-dev] [PATCH mesa 5/5] wayland-egl: Update ABI checker

[Emil Velikov]

On 18 July 2017 at 21:49, Miguel A. Vico <mvicomoya at nvidia.com> wrote:
> This change updates wayland-egl-abi-check.c with the latest changes to
> wl_egl_window.
>
> Signed-off-by: Miguel A. Vico <mvicomoya at nvidia.com>
> Reviewed-by: James Jones <jajones at nvidia.com>
> ---
>  .../wayland/wayland-egl/wayland-egl-abi-check.c    | 78 ++++++++++++++++++----
>  1 file changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> index 1962f05850..6bdd71b6e0 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> +++ b/src/egl/wayland/wayland-egl/wayland-egl-abi-check.c
> @@ -31,7 +31,28 @@
>   * DO NOT EVER CHANGE!
>   */
>
> +/* From: a2ab5c2588 - Miguel A. Vico : wayland-egl: Make wl_egl_window a versioned struct */
Please keep the sha as XXX - we'll update it as the commit lands.

> +#define WL_EGL_WINDOW_VERSION_v3 3
> +struct wl_egl_window_v3 {
> +    const intptr_t version;
> +
> +    int width;
> +    int height;
> +    int dx;
> +    int dy;
> +
> +    int attached_width;
> +    int attached_height;
> +
> +    void *private;
> +    void (*resize_callback)(struct wl_egl_window *, void *);
> +    void (*destroy_window_callback)(void *);
> +
> +    struct wl_surface *surface;
> +};
> +
>  /* From: 690ead4a13 - Stencel, Joanna : egl/wayland-egl: Fix for segfault in dri2_wl_destroy_surface. */
> +#define WL_EGL_WINDOW_VERSION_v2 2
>  struct wl_egl_window_v2 {
>      struct wl_surface *surface;
>
> @@ -123,6 +144,20 @@ struct wl_egl_window_v0 {
>          }                                                                           \
>      } while (0)
>
> +#define CHECK_VERSION(A_VER, B_VER, MATCH)                                          \
> +    do {                                                                            \
> +        if (((MATCH)  && (WL_EGL_WINDOW_VERSION ## A_VER) !=                        \
> +                         (WL_EGL_WINDOW_VERSION ## B_VER)) ||                       \
> +            (!(MATCH) && (WL_EGL_WINDOW_VERSION ## A_VER) >=                        \
> +                         (WL_EGL_WINDOW_VERSION ## B_VER))) {                       \
> +            printf("Backards incompatible change detected!\n   "                    \
> +                   "WL_EGL_WINDOW_VERSION" #A_VER " %s "                            \
> +                   "WL_EGL_WINDOW_VERSION" #B_VER "\n",                             \
> +                   ((MATCH) ? "!=" : ">="));                                        \
> +            return 1;                                                               \
> +        }                                                                           \
> +    } while (0)
> +
Same crazy idea as CHECK_SIZE - worth having separate macros?

>  int main(int argc, char **argv)
>  {
>      /* Check wl_egl_window_v1 ABI against wl_egl_window_v0 */
> @@ -149,19 +184,36 @@ int main(int argc, char **argv)
>
>      CHECK_SIZE(_v1, _v2, FALSE);
>
> -    /* Check wl_egl_window ABI against wl_egl_window_v2 */
> -    CHECK_MEMBER(_v2,, surface,                 surface);
> -    CHECK_MEMBER(_v2,, width,                   width);
> -    CHECK_MEMBER(_v2,, height,                  height);
> -    CHECK_MEMBER(_v2,, dx,                      dx);
> -    CHECK_MEMBER(_v2,, dy,                      dy);
> -    CHECK_MEMBER(_v2,, attached_width,          attached_width);
> -    CHECK_MEMBER(_v2,, attached_height,         attached_height);
> -    CHECK_MEMBER(_v2,, private,                 private);
> -    CHECK_MEMBER(_v2,, resize_callback,         resize_callback);
> -    CHECK_MEMBER(_v2,, destroy_window_callback, destroy_window_callback);
> -
> -    CHECK_SIZE(_v2,, TRUE);
> +    /* Check wl_egl_window_v3 ABI against wl_egl_window_v2 */
> +    CHECK_MEMBER(_v2, _v3, surface,                 version);
Just hit me that with the current CHECK_MEMBER macro changes like the
above will be easier to miss. Or someone will attempt to "correct" it.
The following seem a bit more obvious, imho.

CHECK_RENAMED_MEMBER(_v2, _v3, surface,                 version);
CHECK_MEMBER(_v2, _v3, width);

> +    CHECK_MEMBER(_v2, _v3, width,                   width);
> +    CHECK_MEMBER(_v2, _v3, height,                  height);
> +    CHECK_MEMBER(_v2, _v3, dx,                      dx);
> +    CHECK_MEMBER(_v2, _v3, dy,                      dy);
> +    CHECK_MEMBER(_v2, _v3, attached_width,          attached_width);
> +    CHECK_MEMBER(_v2, _v3, attached_height,         attached_height);
> +    CHECK_MEMBER(_v2, _v3, private,                 private);
> +    CHECK_MEMBER(_v2, _v3, resize_callback,         resize_callback);
> +    CHECK_MEMBER(_v2, _v3, destroy_window_callback, destroy_window_callback);
> +
> +    CHECK_SIZE   (_v2, _v3, FALSE);
> +    CHECK_VERSION(_v2, _v3, FALSE);
> +
Hmm what does the v2 vs v3 version check bring?

Thanks
Emil