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

[Emil Velikov]

On 5 May 2017 at 13:49, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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?
>
Since Chad is already in there, I'll leave those to him.

>>> +#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.
>
I've decided to keep this patch as a dummy move and refactor with another patch.
If you can skim through it (should be in your inbox) that would be appreciated.

Thanks
Emil