David Herrmann <dh.herrm...@gmail.com> writes: > Hi Jan! Hello!
> 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). I don't understand this. What do you mean by "unbound allocation"? That libxkbcommon could, in theory, return a huge format that would exhaust all memory? >> + 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. Hmm, I thought that bus_error_setfv() only fills the error struct. Is there any documentation that desribes how the internal bus_* stuff works? > 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. I don't think that would work. You would have the same code on the client and on the server and that might not be the same xkb configuration. > Thanks > David -- Jan Synacek Software Engineer, Red Hat
signature.asc
Description: PGP signature
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel