[PATCH weston v5 03/14] libweston: add weston_debug API and implementation

[Daniel Stone]

Hi Pekka,

On Fri, 24 Aug 2018 at 09:12, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Thu, 23 Aug 2018 17:53:07 +0100 Daniel Stone <daniel at fooishbar.org> wrote:
> > The users introduced in this patchset already check if the scope is
> > enabled, which bails out if the scope is NULL. Given that, and that I
> > can't see a sensible behaviour if the scope is NULL, I'm inclined to
> > add an assert(scope) at the top of the function as well as
> > documentation. Does that seem reasonable? Would assert(scope) be
> > better or assert(weston_debug_scope_is_enabled(scope))?
>
> after "compositor: offer logs via weston-debug",
> main.c:custom_handler() will call weston_debug_scope_timestamp()
> unchecked.
>
> Relying on libwayland not logging anything until the debug scope has
> been set up seems a bit too fragile to me.
>
> We could forbid calling weston_debug_scope_timestamp() with NULL scope,
> but I think it would be easier if it just accepted NULL by doing
> whatever non-crashy. That would be easier for developers. It could
> simply replace the scope name with "unknown" for instance. The
> resulting string will probably never be used, so it might just make an
> empty string too.

OK, good point. Generally I hate doing nonsensical/non-helpful things
in unexpected conditions we shouldn't be called, but I don't see an
easy way out of this one for now.

> On another matter, I've been wondering if storing the debug scopes in
> weston_compositor is a good thing. We have the logger initialized
> earlier, and quite a lot might happen before weston_compositor is
> created. Also the idea of a circular buffer for after-the-fact
> reporting might benefit from having debug scopes set up early, and the
> command line argument to enable debug scopes from launch.

Yeah, I think we can definitely rework this to push the scope of the
debug stuff up. As noted in
https://gitlab.freedesktop.org/wayland/weston/issues/133 I think it
would be really good to have all this information as early as
possible, and maybe even outliving the compositor. From a libweston
API point of view, having the same API for general logging as we do
for debug makes a huge amount of sense.

Given how invasive that would be, my suggestion would be that we track
that as a follow-up issue. Once we've got all the functionality in
place we can see for sure what the best API/structure would be.

Cheers,
Daniel