[systemd-devel] [PATCH] localed: log xkbcommon errors

[Lennart Poettering]

On Wed, 03.12.14 07:56, Jan Synacek (jsynacek at redhat.com) wrote:

> The errors are prefixed with "libxkbcommon" to provide some context,
> because they are quite confusing without it. With the prefix, we at
> least know where they come from.
> ---
>  src/locale/localed.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/src/locale/localed.c b/src/locale/localed.c
> index 654b291..1d2673a 100644
> --- a/src/locale/localed.c
> +++ b/src/locale/localed.c
> @@ -1009,7 +1009,14 @@ static int method_set_vc_keyboard(sd_bus *bus, sd_bus_message *m, void *userdata
>  
>  #ifdef HAVE_XKBCOMMON
>  static void log_xkb(struct xkb_context *ctx, enum xkb_log_level lvl, const char *format, va_list args) {
> -        /* suppress xkb messages for now */
> +        _cleanup_free_ char *msg = NULL;
> +        const char *fmt;
> +        char prefix[] = "libxkbcommon: ";
> +
> +        fmt = strappenda(prefix, format);
> +        if (vasprintf(&msg, fmt, args) < 0)
> +                (void) log_oom();
> +        log_debug("%s", msg);

If vapsrintf() failed, then you output a NULL pointer. And gcc/libc
actually take the liberty to crash in some conditions when you ask
them to print a NULL string like this. 

>  }
>  
>  static int verify_xkb_rmlvo(const char *model, const char *layout, const char *variant, const char *options) {
> @@ -1092,9 +1099,11 @@ static int method_set_x11_keyboard(sd_bus *bus, sd_bus_message *m, void *userdat
>                          return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
>  
>                  r = verify_xkb_rmlvo(model, layout, variant, options);
> -                if (r < 0)
> -                        log_warning_errno(r, "Cannot compile XKB keymap for new x11 keyboard layout ('%s' / '%s' / '%s' / '%s'): %m",
> -                                          strempty(model), strempty(layout), strempty(variant), strempty(options));
> +                if (r < 0) {
> +                        log_error_errno(r, "Cannot compile XKB keymap for new x11 keyboard layout ('%s' / '%s' / '%s' / '%s'): %m",
> +                                        strempty(model), strempty(layout), strempty(variant), strempty(options));
> +                        return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Cannot compile XKB keymap, refusing");
> +                }
>  
>                  if (free_and_strdup(&c->x11_layout, layout) < 0 ||
>                      free_and_strdup(&c->x11_model, model) < 0 ||

Looks good otherwise.

Lennart

-- 
Lennart Poettering, Red Hat