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

[Miguel Angel Vico]

On Wed, 19 Jul 2017 12:19:30 +0100
Emil Velikov  wrote:

> On 18 July 2017 at 21:49, Miguel A. Vico  wrote:
> > This change updates wayland-egl-abi-check.c with the latest changes to
> > wl_egl_window.
> >
> > Signed-off-by: Miguel A. Vico 
> > Reviewed-by: James Jones 
> > ---
> >  .../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.
> 

Done.

> > +#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?

Yup. Done.

> 
> >  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);

Sure. Done.

> 
> > +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);

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

[Emil Velikov]

On 18 July 2017 at 21:49, Miguel A. Vico  wrote:
> This change updates wayland-egl-abi-check.c with the latest changes to
> wl_egl_window.
>
> Signed-off-by: Miguel A. Vico 
> Reviewed-by: James Jones 
> ---
>  .../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?

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

[Miguel A. Vico]

This change updates wayland-egl-abi-check.c with the latest changes to
wl_egl_window.

Signed-off-by: Miguel A. Vico 
Reviewed-by: James Jones 
---
 .../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 */
+#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)
+
 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);
+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);
+
+/* Check wl_egl_window ABI against wl_egl_window_v3 */
+CHECK_MEMBER(_v3,, version, version);
+CHECK_MEMBER(_v3,, width,   width);
+CHECK_MEMBER(_v3,, height,  height);
+CHECK_MEMBER(_v3,, dx,  dx);
+CHECK_MEMBER(_v3,, dy,  dy);
+CHECK_MEMBER(_v3,, attached_width,  attached_width);
+CHECK_MEMBER(_v3,, attached_height, attached_height);
+CHECK_MEMBER(_v3,, private, private);
+CHECK_MEMBER(_v3,, resize_callback, resize_callback);
+CHECK_MEMBER(_v3,, destroy_window_callback, destroy_window_callback);
+CHECK_MEMBER(_v3,, surface, surface);
+
+CHECK_SIZE