[systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options

[Lennart Poettering]

On Mon, 20.10.14 12:43, Jan Synacek (jsynacek at redhat.com) wrote:

> When setting any of those using set-x11-keymap, check that their values
> are available on the system.
> ---
>  src/locale/localectl.c | 208 +++++++++++++++++++++++++++++++++----------------
>  1 file changed, 139 insertions(+), 69 deletions(-)
> 
> diff --git a/src/locale/localectl.c b/src/locale/localectl.c
> index 3690f9f..0caf467 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
> +};

Hmm, why is this a bitfield? Do I get this patch right and you are
trying to match the passed arguments to all
models/layouts/variants/options all the time? This means if a layout
happens to have the same name as a model then things will be
ambiguous? I really don't like this I must say.

Or mabye I am not following what you are doing here? Can you elaborate?

Also, this validating really be done on the server side, not
on the client side, to take full effect. 

Doing the listing/completion thing on the client-side appears OK since
it's really more a help to the user, nothing that validates stuff. But
if we want to validate input here, we should really do it on the
server-side, in localed, especially as localed and localectl might run
on different systems and not share the same map list.

Lennart

-- 
Lennart Poettering, Red Hat