[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 <eric at engestrom.ch> 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 <eric at engestrom.ch>
> Reviewed-by: Grazvydas Ignotas <notasas at gmail.com>
> Ack-by: Emil Velikov <emil.l.velikov at gmail.com>

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
>