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

[Frank Henigman]

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
- we don't need to implement the hook in every back end

>>  If someone does need to act on the
>> presence or absence or platform-specifics, they can always examine the
>> json.
> Personally I'd just discard the check and use the function
> unconditionally. For platforms that I'm unsure I'll just put a uniform
> stub alike "not-implemented", thus any users parsing the data will
> have clear feedback.
>
> Does that sound reasonable ?
> Emil