Hi Pekka, On Fri, 24 Aug 2018 at 09:12, Pekka Paalanen <ppaala...@gmail.com> wrote: > On Thu, 23 Aug 2018 17:53:07 +0100 Daniel Stone <dan...@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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel