[Mesa-dev] [PATCH 2/7] egl: fold Android logger into main/

[Emil Velikov]

On 4 May 2017 at 20:17, Rob Herring <robh at kernel.org> wrote:
> On Thu, May 4, 2017 at 1:46 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Will allow us to greatly simplify a lot of the code in egllog.c
>
> Okay, because on its own, this is not an improvement.
>
>>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  src/egl/drivers/dri2/egl_dri2.h         |  1 -
>>  src/egl/drivers/dri2/platform_android.c | 34 -------------------------------
>>  src/egl/main/egllog.c                   | 36 +++++++++++++++++++++++++++++++++
>>  3 files changed, 36 insertions(+), 35 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
>> index f16663712d3..b1e224248cc 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.h
>> +++ b/src/egl/drivers/dri2/egl_dri2.h
>> @@ -59,7 +59,6 @@
>>  #include <system/window.h>
>>  #include <hardware/gralloc.h>
>>  #include <gralloc_drm_handle.h>
>> -#include <cutils/log.h>
>>
>>  #endif /* HAVE_ANDROID_PLATFORM */
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
>> index 35f2e5dbe63..e962e8e00dd 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -1030,38 +1030,6 @@ droid_open_device(struct dri2_egl_display *dri2_dpy)
>>     return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
>>  }
>>
>> -/* support versions < JellyBean */
>> -#ifndef ALOGW
>> -#define ALOGW LOGW
>> -#endif
>> -#ifndef ALOGD
>> -#define ALOGD LOGD
>> -#endif
>> -#ifndef ALOGI
>> -#define ALOGI LOGI
>> -#endif
>> -
>> -static void
>> -droid_log(EGLint level, const char *msg)
>> -{
>> -   switch (level) {
>> -   case _EGL_DEBUG:
>> -      ALOGD("%s", msg);
>> -      break;
>> -   case _EGL_INFO:
>> -      ALOGI("%s", msg);
>> -      break;
>> -   case _EGL_WARNING:
>> -      ALOGW("%s", msg);
>> -      break;
>> -   case _EGL_FATAL:
>> -      LOG_FATAL("%s", msg);
>> -      break;
>> -   default:
>> -      break;
>> -   }
>> -}
>> -
>>  static struct dri2_egl_display_vtbl droid_display_vtbl = {
>>     .authenticate = NULL,
>>     .create_window_surface = droid_create_window_surface,
>> @@ -1118,8 +1086,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
>>     const char *err;
>>     int ret;
>>
>> -   _eglSetLogProc(droid_log);
>> -
>>     loader_set_logger(_eglLog);
>>
>>     dri2_dpy = calloc(1, sizeof(*dri2_dpy));
>> diff --git a/src/egl/main/egllog.c b/src/egl/main/egllog.c
>> index c8307482902..9d7e9302341 100644
>> --- a/src/egl/main/egllog.c
>> +++ b/src/egl/main/egllog.c
>> @@ -44,6 +44,23 @@
>>
>>  #include "egllog.h"
>>
>> +#ifdef HAVE_ANDROID_PLATFORM
>> +#define LOG_TAG "EGL-MAIN"
>> +#include <cutils/log.h>
>> +
>> +/* support versions < JellyBean */
>
> < JellyBean support is long gone, so these can be dropped.
>
We have a couple cases [i915 and i965], which we can drop as well right?

>> +#ifndef ALOGW
>> +#define ALOGW LOGW
>> +#endif
>> +#ifndef ALOGD
>> +#define ALOGD LOGD
>> +#endif
>> +#ifndef ALOGI
>> +#define ALOGI LOGI
>> +#endif
>> +
>> +#endif /* HAVE_ANDROID_PLATFORM */
>> +
>>  #define MAXSTRING 1000
>>  #define FALLBACK_LOG_LEVEL _EGL_WARNING
>>
>> @@ -107,7 +124,26 @@ _eglSetLogProc(_EGLLogProc logger)
>>  static void
>>  _eglDefaultLogger(EGLint level, const char *msg)
>>  {
>> +#ifdef HAVE_ANDROID_PLATFORM
>> +   switch (level) {
>> +   case _EGL_DEBUG:
>> +      ALOGD("%s", msg);
>> +      break;
>> +   case _EGL_INFO:
>> +      ALOGI("%s", msg);
>> +      break;
>> +   case _EGL_WARNING:
>> +      ALOGW("%s", msg);
>> +      break;
>> +   case _EGL_FATAL:
>> +      LOG_FATAL("%s", msg);
>> +      break;
>
> This could all be simplified with a LUT for the level:
>
> const int egl2alog[] = { LOG_ERROR, LOG_WARN, LOG_INFO, LOG_DEBUG };
> ALOG(egl2alog[level], LOG_TAG, "%s", msg);
>
> You loose the abort on fatal errors, but you don't have that in the
> !android case. It doesn't really gain you anything aborting.
>
We have a exit(1) in _eglLog, so things should be fine.

Thanks for the suggestions!
Emil