[Spice-devel] [PATCH spice-common 3/4] RFC: Add spice log categories

[Christophe Fergeau]

On Mon, Jun 12, 2017 at 12:19:53PM +0400, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> A log category is defined with SPICE_LOG_CATEGORY(). The macro will
> register a structure with a constructor (and unregister it on
> unloading with a destructor).
> 
> spice_log_init() must be called at initialization time, to set enabled
> categories and add the 'Spice' glib domain to G_MESSAGES_DEBUG (NB: if
> useful, we could have other log domains automatically associated with
> categories and enable them too).
> 
> A category can be enabled with SPICE_DEBUG="cat_name foo*" (*? are
> accepted glob-like wildcards). All categories are enabled with
> SPICE_DEBUG=1 or 'all'. To list available categories, you can run the
> program with SPICE_DEBUG=help.

Not really a review, nor things to change right now, but
hopefully this can map to command-line arguments as well. Why " " as a
separator rather than ":"?

> +void spice_log_category_register(SpiceLogCategory *cat)
> +{
> +    if (!g_slist_find(log_categories, cat)) {
> +        log_categories = g_slist_prepend(log_categories, cat);
> +    }
> +}

This probably could use g_hash_table_add/g_hash_table_contains instead
even though spice_log_init() needs to iterate on all categories anyways.

> +
> +void spice_log_category_unregister(SpiceLogCategory *cat)
> +{
> +    log_categories = g_slist_remove(log_categories, cat);
> +}
> +
> +void spice_log_init(void)
> +{
> +    bool all = false;
> +    const char *env = g_getenv("SPICE_DEBUG");
> +    const char *genv = g_getenv("G_MESSAGES_DEBUG");
> +
> +    if (g_strcmp0(env, "help") == 0) {
> +        g_printerr("SPICE_DEBUG=1 or 'all' to enable all debug\n");
> +        g_printerr("SPICE_DEBUG=\"cat1* cat2\" to enable specific categories\n");
> +        g_printerr("Available Spice debug categories:\n");
> +        for (GSList *l = log_categories; l != NULL; l = g_slist_next(l)) {
> +            SpiceLogCategory *cat = l->data;
> +            g_printerr("%-20s - %s\n", cat->name, cat->desc);
> +        }
> +        return;
> +    }
> +
> +    /* Make sure GLib default log handler will show the debug
> +     * messages. Messing with environment variables like this is
> +     * rather ugly, but done for historical reasons.
> +     */

I believe this is not really true anymore? (the "historical reason" part)
Overall, I believe for external users, this should be fairly similar to
cddd's RFC, except that there is no support for tweaks, and it
integrates with glib logging?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170612/07d6c209/attachment.sig>