[PATCH 3/5] xfree86/os: add xf86DMI stubs

[Peter Hutterer]

On Sat, Dec 07, 2013 at 04:48:48PM +0100, Daniel Martin wrote:
> Add stubs for DMI identifier initialization, cleanup, add and matching.
> 
> Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> ---

[...]

> +
> +/*
> + * Add a DMI id value for specific key to the key value mapping.
> + */
> +void
> +xf86DMIAdd(const char *const key, char *value)
> +{
> +    struct dmi_kv *dst, *tmp;
> +
> +    for (dst = NULL, tmp = &dmi_kv_map[0]; tmp->key; tmp++)
> +        if (strcmp(tmp->key, key) == 0) {
> +            dst = tmp;
> +            break;
> +        }
> +
> +    assert(dst);

use BUG_WARN here instead of assert. or even a normal error message along
the lines of "DMI identifier "foo" is not yet supported".
That shouldn't be something that brings down the server.

also, there's no need for 'tmp', just check for dst->key when the loop
finishes.

> +    dst->value = value;
> +}
> +
> +/*
> + * Match DMI id values against a key value pair pattern. If a key value pair
> + * matches a key and value in dmi_kv_map, return TRUE.
> + */
> +Bool
> +xf86DMIMatchToken(struct xorg_list *patterns,
> +                  int (*compare) (const char *attr, const char *pattern))
> +{
> +    Bool match;
> +    const xf86MatchGroup *group;
> +
> +    match = FALSE;
> +
> +    /* If there are no patterns, accept the match */
> +    if (xorg_list_is_empty(patterns))
> +        return TRUE;

that seems odd and is a configuration error, right?

> +
> +    xorg_list_for_each_entry(group, patterns, entry) {
> +        struct dmi_kv *cur;
> +        const char *key;
> +        const char *value;
> +
> +        /* group->values needs to have 2 values, the key and value. That's
> +         * the result of a MatchDMI "key|value". */
> +        key = group->values ? group->values[0] : NULL;
> +        value = group->values && key ? group->values[1] : NULL;
> +        if (!key || !value)
> +            continue;

a simpler check here would be

           if (!group->values || !group->values[0] || !group->values[1])
                continue;
           key = group->values[0];
           value = group->values[1];

> +
> +        for (cur = &dmi_kv_map[0]; cur->key; cur++) {
> +            if (compare(cur->key, key) != 0)
> +                continue;
> +
> +            if (compare(cur->value, value) != 0)
> +                return FALSE;
> +
> +            match = TRUE;

        just return TRUE to skip the other loop iterations

> +        }
> +    }
> +
> +    return match;
> +}
> +
> +/*
> + * Is the passed string a valid key? (Used within the parser.)
> + */
> +Bool
> +xf86DMIValidKey(const char *const key)

xf86DMIIsValidKey is IMO more self-explanatory.

Cheers,
   Peter