Hi Jan!
On Tue, Nov 25, 2014 at 9:23 AM, Jan Synacek <jsyna...@redhat.com> wrote:
> The errors are prefixed with "libxkbcommon", because they are quite
> confusing. With the prefix, we at least know where they come from.
> ---
> src/locale/localed.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/locale/localed.c b/src/locale/localed.c
> index 4e56382..ea54798 100644
> --- a/src/locale/localed.c
> +++ b/src/locale/localed.c
> @@ -1011,10 +1011,16 @@ 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 *fmt = NULL;
> + sd_bus_error *e;
> +
> + if (asprintf(&fmt, "libxkbcommon: %s", format) < 0)
> + (void) log_oom();
I think you can safely use:
char fmt[LINE_MAX];
snprintf(fmt, sizeof(fmt), "libxkbcommon: %s", format);
e = xkb_context_get_user_data(ctx);
bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
We use LINE_MAX as explicit limit on a lot of these calls (and I think
it makes sense to prevent unbound allocations).
> + e = xkb_context_get_user_data(ctx);
> + bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
> }
>
> -static int verify_xkb_rmlvo(const char *model, const char *layout, const
> char *variant, const char *options) {
> +static int verify_xkb_rmlvo(const char *model, const char *layout, const
> char *variant, const char *options, sd_bus_error *error) {
> const struct xkb_rule_names rmlvo = {
> .model = model,
> .layout = layout,
> @@ -1033,6 +1039,7 @@ static int verify_xkb_rmlvo(const char *model, const
> char *layout, const char *v
> goto exit;
> }
>
> + xkb_context_set_user_data(ctx, (void *)error);
> xkb_context_set_log_fn(ctx, log_xkb);
>
> km = xkb_keymap_new_from_names(ctx, &rmlvo,
> XKB_KEYMAP_COMPILE_NO_FLAGS);
> @@ -1049,7 +1056,7 @@ exit:
> return r;
> }
> #else
> -static int verify_xkb_rmlvo(const char *model, const char *layout, const
> char *variant, const char *options) {
> +static int verify_xkb_rmlvo(const char *model, const char *layout, const
> char *variant, const char *options, sd_bus_error *error) {
> return 0;
> }
> #endif
> @@ -1087,7 +1094,7 @@ static int method_set_x11_keyboard(sd_bus *bus,
> sd_bus_message *m, void *userdat
> (options && !string_is_safe(options)))
> return sd_bus_error_set_errnof(error, -EINVAL,
> "Received invalid keyboard data");
>
> - r = verify_xkb_rmlvo(model, layout, variant, options);
> + r = verify_xkb_rmlvo(model, layout, variant, options, error);
> if (r < 0)
> log_warning("Cannot compile XKB keymap for new x11
> keyboard layout ('%s' / '%s' / '%s' / '%s'): %s",
> strempty(model), strempty(layout),
> strempty(variant), strempty(options), strerror(-r));
I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
libxkbcommon is still not 100% compatible to libxkb (and doesn't
intend to be that, I guess). As we write X11 configs here, I just
continue with a warning.
If you call bus_error_setfv(), then sd-bus will return a method-error
to the caller. However, you also send a method-return in
method_set_x11_keyboard(). You thus end up with 2 calls.
I'm not sure how to solve this. Furthermore, libxkbcommon can print a
lot of informational warnings, and we shouldn't use just the last one.
One idea would be to copy the same logic into localectl. You could
also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get
more information there.
Yes, this would mean compiling the keymap twice, but it's for the sake
of debugging output so I think it's fine.
Thanks
David
> --
> 1.9.3
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
