[systemd-devel] [PATCH] test-hashmap.c first part

[Lennart Poettering]

On Tue, 26.03.13 15:10, Daniel Buch (boogiewasthere at gmail.com) wrote:

> This is first part of hashmap test unit - last part is in
> development. So review is much appreciated

Thanks for picking this up! Much appreciated!

> +#define FOREACH_CONST_CHAR(value, table) \
> +        for (const char **(value) = (table); (value) && *(value); (value)++)
> +
> +static void test_hashmap_foreach_key(void) {
> +        Hashmap *m = hashmap_new(string_hash_func, string_compare_func);
> +        Iterator i;
> +        const char *s;
> +        const char *key_table[] = {"key 1", "key 2", "key 3", "key
> 4", NULL };

We usually use NULSTR_FOREACH for cases like this. i.e. write something
like this:

static const char key_table[] =
        "key 1\0"
        "key 2\0"
        "key 3\0"
        "key 4\0";

and then iterate through it like so:

const char *i;
NULSTR_FOREACH(i, key_table) {
        ...
}

This is nicer to read, nicer to use, more memory efficient and requires
fewer relocations. Not that it would matter much in this specific
contexts, but it still creates this warm and fuzzy feeling... ;-)

> +static void test_hashmap_foreach(void) {
> +        Hashmap *m = hashmap_new(string_hash_func, string_compare_func);
> +        Iterator i;
> +        char *s;
> +        char *val1 = strdup("my val1");
> +        char *val2 = strdup("my val2");
> +        char *val3 = strdup("my val3");
> +        char *val4 = strdup("my val4");
> +        bool value_found[] = { false, false, false, false };

Hmm, so we generally try to avoid doing function calls in
initialization. I.e. declaring variables, and initializing them with
fixed values is OK, but please don't do variable declaration and
function calls in once, that's only a hidden form of mixing variable
declarations and actual code, which we try to avoid.

Also, would be cool to check for OOM with assert_se() here, each time
you allocate something.

Otherwise looks great!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.