Re: [Mesa-dev] [PATCH mesa v2 1/2] egl/display: only detect the platform once

[Eric Engestrom]

On Friday, 2017-06-16 12:10:55 +0300, Grazvydas Ignotas wrote:
> On Fri, Jun 16, 2017 at 1:53 AM, Eric Engestrom  wrote:
> > My refactor missed the fact that `native_platform` is static.
> > Add the proper guard around the detection code, as it might not be
> > necessary, and only print the debug message when a detection was
> > actually performed.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101252
> > Fixes: 7adb9b094894a512c019 ("egl/display: remove unnecessary code and
> >   make it easier to read")
> > Signed-off-by: Eric Engestrom 
> > Reviewed-by: Grazvydas Ignotas 
> > Ack-by: Emil Velikov 
> 
> nit1: usually it's "Acked-by", but there are a few "Ack-by" in git
> history too (2255 vs 3).
> 
[...]
> 
> nit2: you can make it const. Either way, my r-b still stands.

Fixed both and pushed, thanks and sorry it took so long :)

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

Re: [Mesa-dev] [PATCH mesa v2 1/2] egl/display: only detect the platform once

[Grazvydas Ignotas]

On Fri, Jun 16, 2017 at 1:53 AM, Eric Engestrom  wrote:
> My refactor missed the fact that `native_platform` is static.
> Add the proper guard around the detection code, as it might not be
> necessary, and only print the debug message when a detection was
> actually performed.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101252
> Fixes: 7adb9b094894a512c019 ("egl/display: remove unnecessary code and
>   make it easier to read")
> Signed-off-by: Eric Engestrom 
> Reviewed-by: Grazvydas Ignotas 
> Ack-by: Emil Velikov 

nit1: usually it's "Acked-by", but there are a few "Ack-by" in git
history too (2255 vs 3).

> ---
>  src/egl/main/egldisplay.c | 31 +--
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index 2a1f02738d..ffcebb74c2 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -180,24 +180,27 @@ _eglNativePlatformDetectNativeDisplay(void 
> *nativeDisplay)
>  _EGLPlatformType
>  _eglGetNativePlatform(void *nativeDisplay)
>  {
> -   static _EGLPlatformType native_platform;
> -   char *detection_method;
> -
> -   native_platform = _eglGetNativePlatformFromEnv();
> -   detection_method = "environment overwrite";
> +   static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
>
> if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _eglNativePlatformDetectNativeDisplay(nativeDisplay);
> -  detection_method = "autodetected";
> -   }
> +  char *detection_method;

nit2: you can make it const. Either way, my r-b still stands.

>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _EGL_NATIVE_PLATFORM;
> -  detection_method = "build-time configuration";
> -   }
> +  native_platform = _eglGetNativePlatformFromEnv();
> +  detection_method = "environment overwrite";
>
> -   _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
> -   egl_platforms[native_platform].name, detection_method);
> +  if (native_platform == _EGL_INVALID_PLATFORM) {
> + native_platform = 
> _eglNativePlatformDetectNativeDisplay(nativeDisplay);
> + detection_method = "autodetected";
> +  }
> +
> +  if (native_platform == _EGL_INVALID_PLATFORM) {
> + native_platform = _EGL_NATIVE_PLATFORM;
> + detection_method = "build-time configuration";
> +  }
> +
> +  _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
> +  egl_platforms[native_platform].name, detection_method);
> +   }
>
> return native_platform;
>  }
> --
> Cheers,
>   Eric
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH mesa v2 1/2] egl/display: only detect the platform once

[Eric Engestrom]

My refactor missed the fact that `native_platform` is static.
Add the proper guard around the detection code, as it might not be
necessary, and only print the debug message when a detection was
actually performed.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101252
Fixes: 7adb9b094894a512c019 ("egl/display: remove unnecessary code and
  make it easier to read")
Signed-off-by: Eric Engestrom 
Reviewed-by: Grazvydas Ignotas 
Ack-by: Emil Velikov 
---
 src/egl/main/egldisplay.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 2a1f02738d..ffcebb74c2 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -180,24 +180,27 @@ _eglNativePlatformDetectNativeDisplay(void *nativeDisplay)
 _EGLPlatformType
 _eglGetNativePlatform(void *nativeDisplay)
 {
-   static _EGLPlatformType native_platform;
-   char *detection_method;
-
-   native_platform = _eglGetNativePlatformFromEnv();
-   detection_method = "environment overwrite";
+   static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
 
if (native_platform == _EGL_INVALID_PLATFORM) {
-  native_platform = _eglNativePlatformDetectNativeDisplay(nativeDisplay);
-  detection_method = "autodetected";
-   }
+  char *detection_method;
 
-   if (native_platform == _EGL_INVALID_PLATFORM) {
-  native_platform = _EGL_NATIVE_PLATFORM;
-  detection_method = "build-time configuration";
-   }
+  native_platform = _eglGetNativePlatformFromEnv();
+  detection_method = "environment overwrite";
 
-   _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
-   egl_platforms[native_platform].name, detection_method);
+  if (native_platform == _EGL_INVALID_PLATFORM) {
+ native_platform = 
_eglNativePlatformDetectNativeDisplay(nativeDisplay);
+ detection_method = "autodetected";
+  }
+
+  if (native_platform == _EGL_INVALID_PLATFORM) {
+ native_platform = _EGL_NATIVE_PLATFORM;
+ detection_method = "build-time configuration";
+  }
+
+  _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
+  egl_platforms[native_platform].name, detection_method);
+   }
 
return native_platform;
 }
-- 
Cheers,
  Eric

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