On Fri, 20 Jul 2018 20:03:26 +0100
Daniel Stone <[email protected]> wrote:

> From: Pekka Paalanen <[email protected]>
> 
> A tool for accessing the zcompositor_debug_v1 interface features.
> 
> Installed along weston-info, because it should be potentially useful for
> people running libweston-based compositors.
> 
> Signed-off-by: Pekka Paalanen <[email protected]>
> 
> Added a man page for weston-debug client
> 
> Signed-off-by: Maniraj Devadoss <[email protected]>
> [Pekka: fixed 'missing braces aroudn initializer' warning]
> 
> Add --list and --all arguments, using interface advertisement.
> 
> Signed-off-by: Daniel Stone <[email protected]>
> ---
>  Makefile.am            |  16 +-
>  clients/weston-debug.c | 453 +++++++++++++++++++++++++++++++++++++++++
>  man/weston-debug.man   |  46 +++++
>  3 files changed, 513 insertions(+), 2 deletions(-)
>  create mode 100644 clients/weston-debug.c
>  create mode 100644 man/weston-debug.man

...

> diff --git a/clients/weston-debug.c b/clients/weston-debug.c
> new file mode 100644
> index 000000000..59dacd269
> --- /dev/null
> +++ b/clients/weston-debug.c
> @@ -0,0 +1,453 @@
> +/*
> + * Copyright © 2017 Pekka Paalanen <[email protected]>
> + *
> + * 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 <errno.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <getopt.h>
> +#include <assert.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include <wayland-client.h>
> +
> +#include "shared/helpers.h"
> +#include "shared/zalloc.h"
> +#include "weston-debug-client-protocol.h"
> +
> +struct debug_app {
> +     struct {
> +             bool help;
> +             bool list;
> +             bool bind_all;
> +             char *output;
> +             char *outfd;
> +     } opt;
> +
> +     int out_fd;
> +     struct wl_display *dpy;
> +     struct wl_registry *registry;
> +     struct weston_debug_v1 *debug_iface;
> +     struct wl_list stream_list;
> +};
> +
> +struct debug_stream {
> +     struct wl_list link;
> +     bool should_bind;
> +     char *name;
> +     struct weston_debug_stream_v1 *obj;
> +};

...

> +static void
> +debug_advertise(void *data, struct weston_debug_v1 *debug, const char *name)
> +{
> +     struct debug_app *app = data;

Add empty line.

> +     (void) stream_find(app, name);
> +}
> +
> +static const struct weston_debug_v1_listener debug_listener = {
> +     debug_advertise,
> +};

...

> +static int
> +parse_cmdline(struct debug_app *app, int argc, char **argv)
> +{
> +     static const struct option opts[] = {
> +             { "help", no_argument, NULL, 'h' },
> +             { "list", no_argument, NULL, 'l' },
> +             { "all-streams", no_argument, NULL, 'a' },
> +             { "output", required_argument, NULL, 'o' },
> +             { "outfd", required_argument, NULL, 'f' },
> +             { 0 }
> +     };
> +     static const char optstr[] = "hlao:f:";
> +     int c;
> +     bool failed = false;
> +
> +     while (1) {
> +             c = getopt_long(argc, argv, optstr, opts, NULL);
> +             if (c == -1)
> +                     break;
> +
> +             switch (c) {
> +             case 'h':
> +                     app->opt.help = true;
> +                     break;
> +             case 'l':
> +                     app->opt.list = true;
> +                     break;
> +             case 'a':
> +                     app->opt.bind_all = true;
> +                     break;
> +             case 'o':
> +                     free(app->opt.output);
> +                     app->opt.output = strdup(optarg);
> +                     break;
> +             case 'f':
> +                     free(app->opt.outfd);
> +                     app->opt.outfd = strdup(optarg);
> +                     break;
> +             case '?':
> +                     failed = true;
> +                     break;
> +             default:
> +                     fprintf(stderr, "huh? getopt => %c (%d)\n", c, c);
> +                     failed = true;
> +             }
> +     }
> +
> +     if (failed)
> +             return -1;
> +
> +     while (optind < argc) {
> +             struct debug_stream *stream = stream_alloc(app, argv[optind++]);

Add empty line.

> +             stream->should_bind = true;
> +     }
> +
> +     return 0;
> +}


When I tried the tool, I found this example:

$ weston-debug --list akjdakjdhask
Available debug streams:
    akjdakjdhask [will bind]
    scene-graph [will not bind]
    log [will not bind]
    proto [will not bind]
    xwm-wm-x11 [will not bind]
Debug stream 'akjdakjdhask' aborted: Debug stream name 'akjdakjdhask' is 
unknown.

It is confusing to see 'akjdakjdhask' listed as available and then said
to be unknown.


> diff --git a/man/weston-debug.man b/man/weston-debug.man
> new file mode 100644
> index 000000000..3a6a17617
> --- /dev/null
> +++ b/man/weston-debug.man
> @@ -0,0 +1,46 @@
> +.TH WESTON-DEBUG 1 "2017-08-02" "Weston __version__"
> +.SH NAME
> +weston-debug \- a tool for getting debug messages from compositor.
> +.SH SYNOPSIS
> +.B weston-debug [options] [names]
> +.
> +.\" ***************************************************************
> +.SH DESCRIPTION
> +
> +.B weston-debug
> +is a debugging tool which uses weston_debug_v1 interface to get the
> +debug messages from the compositor. The debug messages are categorized into 
> different
> +debug streams by the compositor (example: logs, proto, list, etc.,) and the 
> compositor
> +requires a file descriptor to stream the messages.
> +
> +This tool accepts a file name or a file desciptor (not both) and any desired 
> debug stream
> +names from the user as command line arguments and subscribes the desired 
> streams from the
> +compositor by using the zcompositor_debug_v1 interface. After the 
> subscription, the

It is called weston_debug_v1 in this iteration.


> +compositor will start to write the debug messages to the shared file 
> descriptor.
> +
> +If no file name or file descriptor argument is given, the tool will use the 
> stdout file
> +descriptor. If no debug stream name argument is given, the tool will use the 
> the name "list"
> +which results the names of all the supported debug streams by the compositor.

There is no stream named "list" in the compositor anymore.

> +
> +.
> +.\" ***************************************************************
> +.SH OPTIONS
> +.
> +.B weston-debug
> +accepts the following command line options.
> +.TP
> +. B \-h, \-\-help
> +Print the help text and exit with success.

--list and --all-streams arguments missing.

> +.TP
> +. B \-o FILE, \-\-output FILE
> +Direct output to file named FILE. Use - for stdout.
> +Stdout is the default. Mutually exclusive with -f.
> +.TP
> +. B \-f FD, \-\-outfd FD
> +Direct output to the file descriptor FD.
> +Stdout (1) is the default. Mutually exclusive with -o.
> +.TP
> +.B [names]
> +Names are whatever debug stream names the compositor supports. If none
> +are given, the name "list" is used, to which the compositor should reply
> +with a list of all supported names.

In the code, if no options are given, the tool now exits without
printing anything. How about defaulting to the --list behaviour?

Not sure how understandable the "[will not bind]" messages are when one
simply does 'weston-debug --list'. It sounds a bit like "it's there but
you can't have it". Maybe make it: bind ? " [subscribed]" : "".


Thanks,
pq

Attachment: pgpfFoTKVo954.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to