[waffle] [PATCH 01/12] core: store platform type in wcore_platform

[Chad Versace]

On 01/08/2016 04:00 AM, Emil Velikov wrote:
> Hi Frank,
> 
> On 6 January 2016 at 21:56, Frank Henigman <fjhenigman at google.com> wrote:
>> Facilitates platform-specific code in core functions, like the forthcoming
>> wflinfo-like function.
>>
>> Signed-off-by: Frank Henigman <fjhenigman at google.com>
>> ---
>>  src/waffle/api/waffle_init.c     | 32 +++++++++++++++++++++-----------
>>  src/waffle/core/wcore_platform.h |  1 +
>>  2 files changed, 22 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/waffle/api/waffle_init.c b/src/waffle/api/waffle_init.c
>> index 60091d1..3d8d350 100644
>> --- a/src/waffle/api/waffle_init.c
>> +++ b/src/waffle/api/waffle_init.c
>> @@ -142,43 +142,53 @@ waffle_init_parse_attrib_list(
>>  static struct wcore_platform*
>>  waffle_init_create_platform(int32_t waffle_platform)
>>  {
>> +    struct wcore_platform *wc_platform = NULL;
>> +
>>      switch (waffle_platform) {
>>  #ifdef WAFFLE_HAS_ANDROID
>>          case WAFFLE_PLATFORM_ANDROID:
>> -            return droid_platform_create();
>> +            wc_platform = droid_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_CGL
>>          case WAFFLE_PLATFORM_CGL:
>> -            return cgl_platform_create();
>> +            wc_platform = cgl_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_GLX
>>          case WAFFLE_PLATFORM_GLX:
>> -            return glx_platform_create();
>> +            wc_platform = glx_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_WAYLAND
>>          case  WAFFLE_PLATFORM_WAYLAND:
>> -            return wayland_platform_create();
>> +            wc_platform = wayland_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_X11_EGL
>>          case WAFFLE_PLATFORM_X11_EGL:
>> -            return xegl_platform_create();
>> +            wc_platform = xegl_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_GBM
>>          case WAFFLE_PLATFORM_GBM:
>> -            return wgbm_platform_create();
>> +            wc_platform = wgbm_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_WGL
>>          case WAFFLE_PLATFORM_WGL:
>> -            return wgl_platform_create();
>> +            wc_platform = wgl_platform_create();
>> +            break;
>>  #endif
>>  #ifdef WAFFLE_HAS_NACL
>>          case WAFFLE_PLATFORM_NACL:
>> -            return nacl_platform_create();
>> +            wc_platform = nacl_platform_create();
>> +            break;
>>  #endif
>> -        default:
>> -            assert(false);
>> -            return NULL;

> If we remove the default statement the compiler will shout loudly at
> us. Just initialize wc_platform here ?

Like Emil said, the default is required to eliminate compiler warnings,
as this switch is non-exhaustive. It's best to keep the default statement
as-is (assert for debug builds, return NULL for release builds).

> 
>>      }

An empty line after the brace, please.

>> +    assert(wc_platform);
>> +    wc_platform->waffle_platform = waffle_platform;
>> +    return wc_platform;
>>  }
>>
>>  WAFFLE_API bool
>> diff --git a/src/waffle/core/wcore_platform.h b/src/waffle/core/wcore_platform.h
>> index 780d07a..30d1c6c 100644
>> --- a/src/waffle/core/wcore_platform.h
>> +++ b/src/waffle/core/wcore_platform.h
>> @@ -139,6 +139,7 @@ struct wcore_platform_vtbl {
>>
>>  struct wcore_platform {
>>      const struct wcore_platform_vtbl *vtbl;
>> +    int32_t waffle_platform; // WAFFLE_PLATFORM_* enum

> Worth using the enum here ? (same goes for patch 2)

Yes, this should be an enum. Originally, values like this were
int32_t throughout the waffle code. But then Brian Paul requested that
waffle use enums instead because it made debugging easier. (gdb understands
how to print enum names, but doesn't know the name if the value's type is
a plain int).