[systemd-devel] [PATCH] localed: validate set-x11-keymap input

[Jan Synacek]

Jan Synacek <jsynacek at redhat.com> writes:
> Lennart Poettering <lennart at poettering.net> writes:
>> On Tue, 04.11.14 12:05, Jan Synacek (jsynacek at redhat.com) wrote:
>>> +int xkb_validate_keymaps(const char *model,
>>> +                         const char *layouts_arg,
>>> +                         const char *variants_arg,
>>> +                         const char *options_arg,
>>> +                         char **error)
>>> +{
>>> +        int r;
>>> +
>>> +        if (layouts_arg) {
>>> +                _cleanup_strv_free_ char **layouts_list = NULL;
>>> +                char **it, **layouts;
>>> +                int i = 0;
>>> +
>>> +                r = xkb_get_keymaps(&layouts_list, LAYOUTS, NULL);
>>> +                if (r < 0)
>>> +                        return r;
>>> +
>>> +                layouts = strv_split(layouts_arg, ",");
>>> +                if (!layouts)
>>> +                        return log_oom();
>>> +
>>> +                STRV_FOREACH(it, layouts) {
>>> +                        _cleanup_strv_free_ char **variants_list = NULL;
>>> +                        bool variants_left = true;
>>> +                        int n;
>>> +
>>> +                        if (!strv_find(layouts_list, *it)) {
>>> +                                r = asprintf(error, "Requested layout '%s' not available.\n", *it);
>>> +                                if (r < 0)
>>> +                                        return log_oom();
>>> +                                return -EINVAL;
>>> +                        }
>>> +
>>> +                        if (variants_left && variants_arg) {
>>> +                                _cleanup_strv_free_ char **variants;
>>> +
>>> +                                r = xkb_get_keymaps(&variants_list, VARIANTS, *it);
>>> +                                if (r < 0)
>>> +                                        return r;
>>
>> Hmm, reading the file over and over and over again sounds less than
>> ideal. Maybe we should intrdouce a struct here and then make
>> xkb_get_keymaps() return an array of structs really?
>
> That sounds ok, I'll see what I can do. I wanted to preserve as much of
> the original code as I could, but maybe it wasn't the right decision.

After thinking about it, I can put the layout and its variants into a
structure, but... When parsing "/usr/share/X11/xkb/rules/base.lst", is
it safe to depend on the ordering of all the components in the file? I
mean, is it safe to expect layouts being order before variants in that
file? Or can it be vice versa? The code would have to be more general if
one cannot depend on that order, and I think it would be quite ugly. The
strv implementation doesn't have that problem.

-- 
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/20141111/15a6501d/attachment.sig>