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

[Pekka Paalanen]

On Fri, 20 Jul 2018 20:03:24 +0100
Daniel Stone <daniels at collabora.com> wrote:

> From: Pekka Paalanen <pq at iki.fi>
> 
> weston_debug is both a libweston API for relaying debugging messages,
> and the compositor-debug wayland protocol implementation for accessing those
> debug messages from a Wayland client.
> 
> weston_debug_compositor_{create,destroy}() are private API, hence not
> exported.
> 
> Signed-off-by: Pekka Paalanen <pq at iki.fi>
> 
> append the debug scope name along with the timestamp in
> weston_debug_scope_timestamp API
> 
> Signed-off-by: Maniraj Devadoss <Maniraj.Devadoss at in.bosch.com>
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Add explicit advertisement of debug scope names.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  Makefile.am              |   2 +
>  libweston/compositor.c   |   6 +
>  libweston/compositor.h   |   9 +
>  libweston/weston-debug.c | 723 +++++++++++++++++++++++++++++++++++++++
>  libweston/weston-debug.h | 107 ++++++
>  5 files changed, 847 insertions(+)
>  create mode 100644 libweston/weston-debug.c
>  create mode 100644 libweston/weston-debug.h
> 

...

> diff --git a/libweston/weston-debug.c b/libweston/weston-debug.c
> new file mode 100644
> index 000000000..039247f14
> --- /dev/null
> +++ b/libweston/weston-debug.c
> @@ -0,0 +1,723 @@
> +/*
> + * Copyright © 2017 Pekka Paalanen <pq at iki.fi>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "config.h"
> +
> +#include "weston-debug.h"
> +#include "helpers.h"
> +#include "compositor.h"
> +
> +#include "weston-debug-server-protocol.h"
> +
> +#include <unistd.h>
> +#include <stdarg.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <sys/time.h>
> +
> +/** Main weston-debug context
> + *
> + * One per weston_compositor.
> + *
> + * \internal
> + */
> +struct weston_debug_compositor {
> +	struct weston_compositor *compositor;
> +	struct wl_listener compositor_destroy_listener;
> +	struct wl_global *global;
> +	struct wl_list scope_list; /**< weston_debug_scope::compositor_link */
> +
> +	struct weston_debug_scope *list;

Hi Daniel,

'list' member is now unused, replaced by the explicit advertisement
events.


> +};
> +
> +/** weston-debug message scope
> + *
> + * This is used for scoping debugging messages. Clients can subscribe to
> + * only the scopes they are interested in. A scope is identified by its name
> + * (also referred to as debug stream name).
> + */
> +struct weston_debug_scope {
> +	char *name;
> +	char *desc;

'desc' is never used anymore. This was meant to give the human looking
at the list of debug scopes a description of what they are, but looks
like the protocol does not carry it.

Do we not need it?


> +	weston_debug_scope_cb begin_cb;
> +	void *user_data;
> +	struct wl_list stream_list; /**< weston_debug_stream::scope_link */
> +	struct wl_list compositor_link;
> +};
> +
> +/** A debug stream created by a client
> + *
> + * A client provides a file descriptor for the server to write debug
> + * messages into. A weston_debug_stream is associated to one
> + * weston_debug_scope via the scope name, and the scope provides the messages.
> + * There can be several streams for the same scope, all streams getting the
> + * same messages.
> + */
> +struct weston_debug_stream {
> +	int fd;				/**< client provided fd */
> +	struct wl_resource *resource;	/**< weston_debug_stream_v1 object */
> +	struct wl_list scope_link;
> +};

...

> +static void
> +bind_weston_debug(struct wl_client *client,
> +		   void *data, uint32_t version, uint32_t id)
> +{
> +	struct weston_debug_compositor *wdc = data;
> +	struct weston_debug_scope *scope;
> +	struct wl_resource *resource;
> +
> +	resource = wl_resource_create(client,
> +				      &weston_debug_v1_interface,
> +				      version, id);
> +	if (!resource) {
> +		wl_client_post_no_memory(client);
> +		return;
> +	}
> +	wl_resource_set_implementation(resource, &weston_debug_impl,
> +				       wdc, NULL);
> +
> +       wl_list_for_each(scope, &wdc->scope_list, compositor_link)
> +	       weston_debug_v1_send_available(resource, scope->name);

Whitespace issues in indentation.


Otherwise looks good to me.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180803/a2cbc20f/attachment-0001.sig>