On Sun, Apr 24, 2016 at 5:04 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 24 April 2016 at 20:52, Frank Henigman <fjhenig...@google.com> wrote:
>> On Sun, Apr 24, 2016 at 6:36 AM, Emil Velikov <emil.l.veli...@gmail.com>
>> wrote:
>>> On 21 April 2016 at 21:26, Frank Henigman <fjhenig...@google.com> wrote:
>>>> On Fri, Jan 8, 2016 at 7:44 AM, Emil Velikov <emil.l.veli...@gmail.com>
>>>> wrote:
>>>>> On 6 January 2016 at 21:56, Frank Henigman <fjhenig...@google.com> wrote:
>>>>>> Add a platform hook so platform-specific information can be included
>>>>>> by waffle_display_info_json().
>>>>>>
>>>>>> Signed-off-by: Frank Henigman <fjhenig...@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.
That's true - when memory runs out the whole string is lost. I guess
that's the trade off for the simplicity of the error handling. I'm
quite happy with that trade off.
> 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 ;-)
I don't want to have to try to land code (even a stub) in back ends
which I've never even compiled (and which would be a pain in the butt
to set up for compiling). Waffle needs continuous integration. (:
_______________________________________________
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle
