[Mesa-dev] [PATCH 1/2] egl: Convert int to attrib in eglGetPlatformDisplay

[Adam Jackson]

... because converting attrib to int truncates, and that's bad.

Signed-off-by: Adam Jackson 
---
 src/egl/main/eglapi.c | 29 ++---
 src/egl/main/egldisplay.c | 15 ---
 src/egl/main/egldisplay.h |  8 
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index c1bf5bbfe1..2a6513a95c 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -376,7 +376,7 @@ eglGetDisplay(EGLNativeDisplayType nativeDisplay)
 
 static EGLDisplay
 _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
- const EGLint *attrib_list)
+ const EGLAttrib *attrib_list)
 {
_EGLDisplay *dpy;
 
@@ -412,30 +412,29 @@ _eglGetPlatformDisplayCommon(EGLenum platform, void 
*native_display,
 
 static EGLDisplay EGLAPIENTRY
 eglGetPlatformDisplayEXT(EGLenum platform, void *native_display,
- const EGLint *attrib_list)
-{
-   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
-   return _eglGetPlatformDisplayCommon(platform, native_display, attrib_list);
-}
-
-EGLDisplay EGLAPIENTRY
-eglGetPlatformDisplay(EGLenum platform, void *native_display,
-  const EGLAttrib *attrib_list)
+ const EGLint *int_attribs)
 {
+   EGLAttrib *attrib_list = NULL;
EGLDisplay display;
-   EGLint *int_attribs;
 
_EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
 
-   int_attribs = _eglConvertAttribsToInt(attrib_list);
-   if (attrib_list && !int_attribs)
+   if (_eglConvertIntsToAttribs(int_attribs, &attrib_list) != EGL_SUCCESS)
   RETURN_EGL_ERROR(NULL, EGL_BAD_ALLOC, NULL);
 
-   display = _eglGetPlatformDisplayCommon(platform, native_display, 
int_attribs);
-   free(int_attribs);
+   display = _eglGetPlatformDisplayCommon(platform, native_display, 
attrib_list);
+   free(attrib_list);
return display;
 }
 
+EGLDisplay EGLAPIENTRY
+eglGetPlatformDisplay(EGLenum platform, void *native_display,
+  const EGLAttrib *attrib_list)
+{
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
+   return _eglGetPlatformDisplayCommon(platform, native_display, attrib_list);
+}
+
 /**
  * Copy the extension into the string and update the string pointer.
  */
diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 690728d2f7..fe963c805e 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -447,7 +447,8 @@ _eglUnlinkResource(_EGLResource *res, _EGLResourceType type)
 
 #ifdef HAVE_X11_PLATFORM
 static EGLBoolean
-_eglParseX11DisplayAttribList(_EGLDisplay *display, const EGLint *attrib_list)
+_eglParseX11DisplayAttribList(_EGLDisplay *display,
+  const EGLAttrib *attrib_list)
 {
int i;
 
@@ -456,8 +457,8 @@ _eglParseX11DisplayAttribList(_EGLDisplay *display, const 
EGLint *attrib_list)
}
 
for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
-  EGLint attrib = attrib_list[i];
-  EGLint value = attrib_list[i + 1];
+  EGLAttrib attrib = attrib_list[i];
+  EGLAttrib value = attrib_list[i + 1];
 
   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
@@ -473,7 +474,7 @@ _eglParseX11DisplayAttribList(_EGLDisplay *display, const 
EGLint *attrib_list)
 
 _EGLDisplay*
 _eglGetX11Display(Display *native_display,
-  const EGLint *attrib_list)
+  const EGLAttrib *attrib_list)
 {
_EGLDisplay *display = _eglFindDisplay(_EGL_PLATFORM_X11,
   native_display);
@@ -494,7 +495,7 @@ _eglGetX11Display(Display *native_display,
 #ifdef HAVE_DRM_PLATFORM
 _EGLDisplay*
 _eglGetGbmDisplay(struct gbm_device *native_display,
-  const EGLint *attrib_list)
+  const EGLAttrib *attrib_list)
 {
/* EGL_MESA_platform_gbm recognizes no attributes. */
if (attrib_list != NULL && attrib_list[0] != EGL_NONE) {
@@ -509,7 +510,7 @@ _eglGetGbmDisplay(struct gbm_device *native_display,
 #ifdef HAVE_WAYLAND_PLATFORM
 _EGLDisplay*
 _eglGetWaylandDisplay(struct wl_display *native_display,
-  const EGLint *attrib_list)
+  const EGLAttrib *attrib_list)
 {
/* EGL_EXT_platform_wayland recognizes no attributes. */
if (attrib_list != NULL && attrib_list[0] != EGL_NONE) {
@@ -524,7 +525,7 @@ _eglGetWaylandDisplay(struct wl_display *native_display,
 #ifdef HAVE_SURFACELESS_PLATFORM
 _EGLDisplay*
 _eglGetSurfacelessDisplay(void *native_display,
-  const EGLint *attrib_list)
+  const EGLAttrib *attrib_list)
 {
/* This platform has no native display. */
if (native_display != NULL) {
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index 0f57680b21..52fb4ce21b 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main

Re: [Mesa-dev] [PATCH 1/2] egl: Convert int to attrib in eglGetPlatformDisplay

[Eric Engestrom]

On Thursday, 2017-11-16 13:27:27 -0500, Adam Jackson wrote:
> ... because converting attrib to int truncates, and that's bad.
> 
> Signed-off-by: Adam Jackson 
> ---
>  src/egl/main/eglapi.c | 29 ++---
>  src/egl/main/egldisplay.c | 15 ---
>  src/egl/main/egldisplay.h |  8 
>  3 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index c1bf5bbfe1..2a6513a95c 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -376,7 +376,7 @@ eglGetDisplay(EGLNativeDisplayType nativeDisplay)
>  
>  static EGLDisplay
>  _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
> - const EGLint *attrib_list)
> + const EGLAttrib *attrib_list)
>  {
> _EGLDisplay *dpy;
>  
> @@ -412,30 +412,29 @@ _eglGetPlatformDisplayCommon(EGLenum platform, void 
> *native_display,
>  
>  static EGLDisplay EGLAPIENTRY
>  eglGetPlatformDisplayEXT(EGLenum platform, void *native_display,
> - const EGLint *attrib_list)
> -{
> -   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
> -   return _eglGetPlatformDisplayCommon(platform, native_display, 
> attrib_list);
> -}
> -
> -EGLDisplay EGLAPIENTRY
> -eglGetPlatformDisplay(EGLenum platform, void *native_display,
> -  const EGLAttrib *attrib_list)
> + const EGLint *int_attribs)
>  {
> +   EGLAttrib *attrib_list = NULL;

Nit: no need to initialise, _eglConvertIntsToAttribs() will always set
it when returning EGL_SUCCESS.

Reviewed-by: Eric Engestrom 

> EGLDisplay display;
> -   EGLint *int_attribs;
>  
> _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
>  
> -   int_attribs = _eglConvertAttribsToInt(attrib_list);
> -   if (attrib_list && !int_attribs)
> +   if (_eglConvertIntsToAttribs(int_attribs, &attrib_list) != EGL_SUCCESS)
>RETURN_EGL_ERROR(NULL, EGL_BAD_ALLOC, NULL);
>  
> -   display = _eglGetPlatformDisplayCommon(platform, native_display, 
> int_attribs);
> -   free(int_attribs);
> +   display = _eglGetPlatformDisplayCommon(platform, native_display, 
> attrib_list);
> +   free(attrib_list);
> return display;
>  }
>  
> +EGLDisplay EGLAPIENTRY
> +eglGetPlatformDisplay(EGLenum platform, void *native_display,
> +  const EGLAttrib *attrib_list)
> +{
> +   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
> +   return _eglGetPlatformDisplayCommon(platform, native_display, 
> attrib_list);
> +}
> +
>  /**
>   * Copy the extension into the string and update the string pointer.
>   */
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index 690728d2f7..fe963c805e 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -447,7 +447,8 @@ _eglUnlinkResource(_EGLResource *res, _EGLResourceType 
> type)
>  
>  #ifdef HAVE_X11_PLATFORM
>  static EGLBoolean
> -_eglParseX11DisplayAttribList(_EGLDisplay *display, const EGLint 
> *attrib_list)
> +_eglParseX11DisplayAttribList(_EGLDisplay *display,
> +  const EGLAttrib *attrib_list)
>  {
> int i;
>  
> @@ -456,8 +457,8 @@ _eglParseX11DisplayAttribList(_EGLDisplay *display, const 
> EGLint *attrib_list)
> }
>  
> for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
> -  EGLint attrib = attrib_list[i];
> -  EGLint value = attrib_list[i + 1];
> +  EGLAttrib attrib = attrib_list[i];
> +  EGLAttrib value = attrib_list[i + 1];
>  
>/* EGL_EXT_platform_x11 recognizes exactly one attribute,
> * EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
> @@ -473,7 +474,7 @@ _eglParseX11DisplayAttribList(_EGLDisplay *display, const 
> EGLint *attrib_list)
>  
>  _EGLDisplay*
>  _eglGetX11Display(Display *native_display,
> -  const EGLint *attrib_list)
> +  const EGLAttrib *attrib_list)
>  {
> _EGLDisplay *display = _eglFindDisplay(_EGL_PLATFORM_X11,
>native_display);
> @@ -494,7 +495,7 @@ _eglGetX11Display(Display *native_display,
>  #ifdef HAVE_DRM_PLATFORM
>  _EGLDisplay*
>  _eglGetGbmDisplay(struct gbm_device *native_display,
> -  const EGLint *attrib_list)
> +  const EGLAttrib *attrib_list)
>  {
> /* EGL_MESA_platform_gbm recognizes no attributes. */
> if (attrib_list != NULL && attrib_list[0] != EGL_NONE) {
> @@ -509,7 +510,7 @@ _eglGetGbmDisplay(struct gbm_device *native_display,
>  #ifdef HAVE_WAYLAND_PLATFORM
>  _EGLDisplay*
>  _eglGetWaylandDisplay(struct wl_display *native_display,
> -  const EGLint *attrib_list)
> +  const EGLAttrib *attrib_list)
>  {
> /* EGL_EXT_platform_wayland recognizes no attributes. */
> if (attrib_list != NULL && attrib_list[0] != EGL_NONE) {
> @@ -524,7 +525,7 @@ _eglGetWaylandDisplay(struct wl_display *native_

Re: [Mesa-dev] [PATCH 1/2] egl: Convert int to attrib in eglGetPlatformDisplay

[Emil Velikov]

On 16 November 2017 at 18:27, Adam Jackson  wrote:
> ... because converting attrib to int truncates, and that's bad.
>
I seems to recall mentioning the same thing as _eglConvertIntsToAttribs
Indeed truncating is bad - fortunately we don't have a case where that matters..

With Eric's comment the patch is
Reviewed-by: Emil Velikov 

Aside: I'll update the other similar cases and drop the truncating
helper in a second.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev