[systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options
Ran Benita
ran234 at gmail.com
Fri Oct 17 07:01:33 PDT 2014
On Fri, Oct 17, 2014 at 02:02:13PM +0200, Jan Synacek wrote:
> When setting any of those using set-x11-keymap, check that their values
> are available on the system.
I have only skimmed this patch, but generally:
1. There can only be one model.
2. There can be multiple layouts and variants, comma separated. E.g.,
layout=us,il variant=,lyx. The amount of layouts and variants should
match (or the variants can be empty), but most stuff you throw at it
will be accepted though.
3. There can be multiple comma-separated options.
Do you handle input like "layout=us,il variant=,lyx" correctly?
Ran
> ---
> src/locale/localectl.c | 179 ++++++++++++++++++++++++++++++-------------------
> 1 file changed, 110 insertions(+), 69 deletions(-)
>
> diff --git a/src/locale/localectl.c b/src/locale/localectl.c
> index 3690f9f..79bc2d0 100644
> --- a/src/locale/localectl.c
> +++ b/src/locale/localectl.c
> @@ -53,6 +53,14 @@ static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
> static char *arg_host = NULL;
> static bool arg_convert = true;
>
> +enum keymap_state {
> + NONE,
> + MODELS = 1 << 0,
> + LAYOUTS = 1 << 1,
> + VARIANTS = 1 << 2,
> + OPTIONS = 1 << 3
> +};
> +
> static void pager_open_if_enabled(void) {
>
> if (arg_no_pager)
> @@ -350,59 +358,12 @@ static int list_vconsole_keymaps(sd_bus *bus, char **args, unsigned n) {
> return 0;
> }
>
> -static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
> - _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
> - const char *layout, *model, *variant, *options;
> - int r;
> -
> - assert(bus);
> - assert(args);
> -
> - if (n > 5) {
> - log_error("Too many arguments.");
> - return -EINVAL;
> - }
> -
> - polkit_agent_open_if_enabled();
> -
> - layout = args[1];
> - model = n > 2 ? args[2] : "";
> - variant = n > 3 ? args[3] : "";
> - options = n > 4 ? args[4] : "";
> -
> - r = sd_bus_call_method(
> - bus,
> - "org.freedesktop.locale1",
> - "/org/freedesktop/locale1",
> - "org.freedesktop.locale1",
> - "SetX11Keyboard",
> - &error,
> - NULL,
> - "ssssbb", layout, model, variant, options,
> - arg_convert, arg_ask_password);
> - if (r < 0)
> - log_error("Failed to set keymap: %s", bus_error_message(&error, -r));
> -
> - return r;
> -}
> -
> -static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
> +static int get_x11_keymaps_internal(char ***list, enum keymap_state look_for, const char *layout)
> +{
> _cleanup_fclose_ FILE *f = NULL;
> - _cleanup_strv_free_ char **list = NULL;
> char line[LINE_MAX];
> - enum {
> - NONE,
> - MODELS,
> - LAYOUTS,
> - VARIANTS,
> - OPTIONS
> - } state = NONE, look_for;
> - int r;
> -
> - if (n > 2) {
> - log_error("Too many arguments.");
> - return -EINVAL;
> - }
> + enum keymap_state state = NONE;
> + int r = 0;
>
> f = fopen("/usr/share/X11/xkb/rules/base.lst", "re");
> if (!f) {
> @@ -410,17 +371,6 @@ static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
> return -errno;
> }
>
> - if (streq(args[0], "list-x11-keymap-models"))
> - look_for = MODELS;
> - else if (streq(args[0], "list-x11-keymap-layouts"))
> - look_for = LAYOUTS;
> - else if (streq(args[0], "list-x11-keymap-variants"))
> - look_for = VARIANTS;
> - else if (streq(args[0], "list-x11-keymap-options"))
> - look_for = OPTIONS;
> - else
> - assert_not_reached("Wrong parameter");
> -
> FOREACH_LINE(line, f, break) {
> char *l, *w;
>
> @@ -444,12 +394,12 @@ static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
> continue;
> }
>
> - if (state != look_for)
> + if (!(state & look_for))
> continue;
>
> w = l + strcspn(l, WHITESPACE);
>
> - if (n > 1) {
> + if (layout) {
> char *e;
>
> if (*w == 0)
> @@ -465,23 +415,114 @@ static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
>
> *e = 0;
>
> - if (!streq(w, args[1]))
> + if (!streq(w, layout))
> continue;
> } else
> *w = 0;
>
> - r = strv_extend(&list, l);
> + r = strv_extend(list, l);
> if (r < 0)
> return log_oom();
> }
>
> - if (strv_isempty(list)) {
> + if (strv_isempty(*list)) {
> log_error("Couldn't find any entries.");
> return -ENOENT;
> }
>
> - strv_sort(list);
> - strv_uniq(list);
> + strv_sort(*list);
> + strv_uniq(*list);
> +
> + return r;
> +}
> +
> +static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
> + _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
> + _cleanup_strv_free_ char **list = NULL;
> + const char *layout, *model, *variant, *options;
> + enum keymap_state look_for;
> + int r;
> +
> + assert(bus);
> + assert(args);
> +
> + if (n > 5) {
> + log_error("Too many arguments.");
> + return -EINVAL;
> + }
> +
> + polkit_agent_open_if_enabled();
> +
> + layout = args[1];
> + model = n > 2 ? args[2] : "";
> + variant = n > 3 ? args[3] : "";
> + options = n > 4 ? args[4] : "";
> +
> + look_for = LAYOUTS;
> + look_for |= !streq(model, "") * MODELS |
> + !streq(variant, "") * VARIANTS |
> + !streq(options, "") * OPTIONS;
> +
> + r = get_x11_keymaps_internal(&list, look_for, NULL);
> + if (!strv_find(list, layout)) {
> + fprintf(stderr, "Requested layout '%s' not available.\n", layout);
> + return -EINVAL;
> + }
> + if ((look_for & MODELS) && !strv_find(list, model)) {
> + fprintf(stderr, "Requested model '%s' not available.\n", model);
> + return -EINVAL;
> + }
> + if ((look_for & VARIANTS) && !strv_find(list, variant)) {
> + fprintf(stderr, "Requested variant '%s' not available.\n", variant);
> + return -EINVAL;
> + }
> + if ((look_for & OPTIONS) && !strv_find(list, options)) {
> + fprintf(stderr, "Requested options '%s' not available.\n", options);
> + return -EINVAL;
> + }
> +
> + r = sd_bus_call_method(
> + bus,
> + "org.freedesktop.locale1",
> + "/org/freedesktop/locale1",
> + "org.freedesktop.locale1",
> + "SetX11Keyboard",
> + &error,
> + NULL,
> + "ssssbb", layout, model, variant, options,
> + arg_convert, arg_ask_password);
> + if (r < 0)
> + log_error("Failed to set keymap: %s", bus_error_message(&error, -r));
> +
> + return r;
> +}
> +
> +static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
> + _cleanup_strv_free_ char **list = NULL;
> + const char *layout;
> + enum keymap_state look_for;
> + int r;
> +
> + if (n > 2) {
> + log_error("Too many arguments.");
> + return -EINVAL;
> + }
> +
> + if (streq(args[0], "list-x11-keymap-models"))
> + look_for = MODELS;
> + else if (streq(args[0], "list-x11-keymap-layouts"))
> + look_for = LAYOUTS;
> + else if (streq(args[0], "list-x11-keymap-variants"))
> + look_for = VARIANTS;
> + else if (streq(args[0], "list-x11-keymap-options"))
> + look_for = OPTIONS;
> + else
> + assert_not_reached("Wrong parameter");
> +
> + layout = (n == 2) ? args[1] : NULL;
> + r = get_x11_keymaps_internal(&list, look_for, layout);
> + if (r < 0)
> + return r;
>
> pager_open_if_enabled();
>
> --
> 1.9.3
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list