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

[Jan Synacek]

Lennart Poettering <lennart at poettering.net> writes:
> On Fri, 14.11.14 12:42, Jan Synacek (jsynacek at redhat.com) wrote:
>> +        if (look_for == LAYOUTS) {
>> +                Set *s;
>> +                char *k;
>> +                Iterator i;
>> +                /* XXX: Is there a better way to sort Hashmap keys? */
>> +                _cleanup_strv_free_ char **tmp = NULL;
>> +
>> +                HASHMAP_FOREACH_KEY(s, k, keymap->x11_layouts, i)
>> +                        if (strv_extend(&tmp, k) < 0)
>> +                                (void) log_oom();
>
> There's hashmap_get_strv() for cases like this.

I need keys, not values. I didn't find any hashmap_get_strv() equivalent
for keys.

>
> Also, please neer invoke strv_extend() when appending to unbounded
> arrays. It's slow! 

What is a preffered way to extend a strv? In this case, I know the final
size, so it the array could manually be copied. I don't think that
that's very nice, though.

>> +void xkb_keymap_free_components(X11Keymap *keymap) {
>> +        if (keymap->x11_layouts) {
>> +                Set *s;
>> +                char *k;
>> +                Iterator i;
>> +
>> +                HASHMAP_FOREACH_KEY(s, k, keymap->x11_layouts, i) {
>> +                        free(k);
>> +                        set_free_free(s);
>> +                }
>
> Humm, when clearing up hashmaps please just write a loop that steals
> the first entry, free it and then repeat. Iterating through a hashmap
> while deleting its entries is unnecessary...

I need to free its entries *and* keys. I didn't find any function that I
could use for that, apart from hashmap_free_free(), which crashes in my case.

> Lennart
>
> -- 
> Lennart Poettering, Red Hat

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/20141124/9901e619/attachment.sig>