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

[Lennart Poettering]

On Thu, 23.10.14 16:22, Jan Synacek (jsynacek at redhat.com) wrote:

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

Well, one option would be to make the function take all four items and
check them all in one go.

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

I think we should just move the parsing/validation of the maps file
from localectl into some file src/share/xkb-util.[ch] or so, and then
use it for listing things on the client side, and for validation on
the server side.

Something similar we already did for the locales stuff which we moved
to src/share/locale-util.[ch] (though that was arguably a simpler
case, since returning a simple strv for the locales works well...)

Lennart

-- 
Lennart Poettering, Red Hat