[waffle] [PATCH 07/12] waffle: support platform-specific information

[Emil Velikov]

On 24 April 2016 at 20:52, Frank Henigman <fjhenigman at google.com> wrote:
> On Sun, Apr 24, 2016 at 6:36 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 21 April 2016 at 21:26, Frank Henigman <fjhenigman at google.com> wrote:
>>> On Fri, Jan 8, 2016 at 7:44 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 6 January 2016 at 21:56, Frank Henigman <fjhenigman at google.com> wrote:
>>>>> Add a platform hook so platform-specific information can be included
>>>>> by waffle_display_info_json().
>>>>>
>>>>> Signed-off-by: Frank Henigman <fjhenigman at google.com>
>>>>> ---
>>>>>  src/waffle/api/waffle_display.c  | 10 +++++++++-
>>>>>  src/waffle/core/wcore_platform.h |  4 ++++
>>>>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/waffle/api/waffle_display.c b/src/waffle/api/waffle_display.c
>>>>> index 7abe2ef..d6988ac 100644
>>>>> --- a/src/waffle/api/waffle_display.c
>>>>> +++ b/src/waffle/api/waffle_display.c
>>>>> @@ -367,8 +367,16 @@ waffle_display_info_json(struct waffle_display *self, bool platform_too)
>>>>>
>>>>>      json_appendv(jj, "{", "generic", "{", "");
>>>>>      add_generic_info(jj, wc_self->current_context);
>>>>> -    json_appendv(jj, "}", "}", "");
>>>>> +    json_append(jj, "}");
>>>>>
>>>>> +    if (platform_too) {
>>>>> +        json_appendv(jj, "platform", "{", "");
>>>>> +        if (api_platform->vtbl->display.info_json)
>>>>> +            api_platform->vtbl->display.info_json(wc_self, jj);
>>>> The rest of waffle tends to set UNSUPPORTED_ON_PLATFORM if the backend
>>>> is missing the vfunc.
>>>
>>> I'm reluctant to set an error for something that isn't clearly an
>>> error (it might just be that a backend doesn't have platform-specific
>>> details) because it could add a burden to the user to distinguish this
>>> case from definite errors.
>> With all respect I have to disagree. Error checking/handling is not a
>> 'burden'. Even if some choose to ignore it that doesn't make it less
>> relevant ;-)
>>
>> Obviously things would be way easier/cleaner if things were split -
>> generic info vs platform specific (or even finer). As-is, with all of
>> them in one piece, no error-checking or UNSUPPORTED_ON_PLATFORM one
>> gets badly formatted/corrupted json. Thus the user has no way of
>> knowing if things failed for reason some, or the setup simply lacks
>> the information Y that they need.
>> </minirant>
>
> You never get corrupt json.  If the hook isn't implemented you get
> different json.  In v1 you'd get an empty platform section.  In v2 the
> platform section (e.g. "glx" or "egl") is omitted.  This is better
> because:
> - the json consumer is in the best position to decide what to do about
> a missing platform section - the api shouldn't decide it's an error
> - the caller of waffle_display_info_json() doesn't have to check
> waffle error state to know if there was a "real" error, they'll know
> by the NULL return
So when someone gets an OOM (even if it's during the platform
specifics) NULL is returned ? That does sound a bit strange, yet again
I would not read too much into it.

As long as others are happy go with it.

> - we don't need to implement the hook in every back end
>
Need, perhaps not. It would be pretty good though ;-)

Thanks
Emil