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

[Jan Synacek]

Lennart Poettering <lennart at poettering.net> writes:

> 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.

Basic idea was that get_x11_keymaps_internal() gets a flag that says
what kind of information should be parsed and returned from
/usr/share/X11/xkb/rules/base.lst. If only a layout needs to be
specified, then fetch just layouts. If a layout, model and options all
get specified, fetch all of them in one go. If I left the function as it
was, it would require multiple passes to validate layouts, models and
options together.

And yes, there's ambiguity in that solution. I didn't come up with
anything better, I'm not even sure if this has a proper solution without
using X libraries, which is IMO unthinkable in this case. If you have a
better idea, I'll be happy to rework this solution with something
better.

> 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.

Doing this on the server side makes sense. It didn't occur to me
before. I saw code that already parsed the local xkb rules, so I just
improved on that.

Also, in case of a failed validation, it's probably better to just warn
the user and go on, as mentioned earlier in this thread.

So, if would it make sense to implement the server-side validation
functionality? Would the DBus API need to be changed? Any other ideas?

> Lennart

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20141023/cd0e5e02/attachment.sig>