[v2 6/6] xfree86: add MatchDMI support in InputClass sections
Peter Hutterer
peter.hutterer at who-t.net
Tue Dec 17 18:42:54 PST 2013
On Tue, Dec 17, 2013 at 11:35:59PM +0100, Daniel Martin wrote:
> This adds support for MatchDMI entries in InputClass sections.
>
> It can be used to match on DMI identifiers applied to the maschine. This
> might be usefull when it's not sufficient or possible to match on device
> tags or something similar.
>
> One has to add a 'MatchDMI "key:value"' line into the InputClass section
> to make use of it. 'key' is a DMI identifier and 'value' the desired
> value. All supported/valid keys are listed in the man-page.
>
> It's possible to have multiple key-value pairs in one MatchDMI line.
> key-value pairs are OR'ed together with a '|' character.
>
> An example:
> MatchDMI "system_vendor:LENOVO"
> MatchDMI "product_name:ThinkPad T440p|product_name:ThinkPad T440s"
>
> Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> ---
> hw/xfree86/common/xf86Xinput.c | 7 ++++
> hw/xfree86/man/xorg.conf.man | 28 ++++++++++++++
> hw/xfree86/parser/InputClass.c | 85 ++++++++++++++++++++++++++++++++++++++++++
> hw/xfree86/parser/xf86Parser.h | 11 ++++++
> hw/xfree86/parser/xf86tokens.h | 1 +
> 5 files changed, 132 insertions(+)
>
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index cdc4d0f..e1a973b 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -56,6 +56,7 @@
> #include <X11/extensions/XIproto.h>
> #include <X11/Xatom.h>
> #include "xf86.h"
> +#include "xf86_OSlib.h"
> #include "xf86Priv.h"
> #include "xf86Config.h"
> #include "xf86Xinput.h"
> @@ -594,6 +595,12 @@ InputClassMatches(const XF86ConfInputClassPtr iclass, const InputInfoPtr idev,
> return FALSE;
>
> /*
> + * MatchDMI string
> + */
> + if (!xf86DMIMatchToken(&iclass->match_dmi, strcmp))
> + return FALSE;
> +
> + /*
> * MatchTag string
> * See if any of the device's tags match any of the MatchTag tokens.
> */
> diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
> index 85f9f2e..c233598 100644
> --- a/hw/xfree86/man/xorg.conf.man
> +++ b/hw/xfree86/man/xorg.conf.man
> @@ -1133,6 +1133,34 @@ been set by the config backend or a previous
> .B InputClass
> section.
> .TP 7
> +.BI "MatchDMI \*q" matchdmi \*q
> +This entry can be used to check if DMI identifiers assigned to the maschine
> +match the
> +.RI \*q matchdmi \*q
> +pattern.
> +.RI \*q matchdmi \*q
> +can be one or more key-value pairs. Pairs have to be separated with a '|'
> +character, the key and value in a pair are separated by a ':' character,
> +i.e.
> +.RI \*q key1:value1|key2:value2 \*q.
> +Possible keys are
> +.B bios_date,
> +.B bios_vendor,
> +.B bios_version,
> +.B board_name,
> +.B board_vendor,
> +.B board_version,
> +.B chassis_type,
> +.B chassis_vendor,
> +.B chassis_version,
> +.B product_name,
> +.B product_version
> +and
> +.B system_vendor.
> +A match is found if at least one of the pairs given in
> +.RI \*q matchdmi \*q
> +matches on DMI identifiers assigned to the maschine.
> +.TP 7
> .BI "MatchTag \*q" matchtag \*q
> This entry can be used to check if tags assigned by the config backend
> matches the
> diff --git a/hw/xfree86/parser/InputClass.c b/hw/xfree86/parser/InputClass.c
> index 24a1246..ea0bc29 100644
> --- a/hw/xfree86/parser/InputClass.c
> +++ b/hw/xfree86/parser/InputClass.c
> @@ -29,6 +29,7 @@
>
> #include <string.h>
> #include "os.h"
> +#include "../os-support/xf86_OSlib.h"
> #include "xf86Parser.h"
> #include "xf86tokens.h"
> #include "Configint.h"
> @@ -47,6 +48,7 @@ xf86ConfigSymTabRec InputClassTab[] = {
> {MATCH_PNPID, "matchpnpid"},
> {MATCH_USBID, "matchusbid"},
> {MATCH_DRIVER, "matchdriver"},
> + {MATCH_DMI, "matchdmi"},
> {MATCH_TAG, "matchtag"},
> {MATCH_LAYOUT, "matchlayout"},
> {MATCH_IS_KEYBOARD, "matchiskeyboard"},
> @@ -61,6 +63,7 @@ xf86ConfigSymTabRec InputClassTab[] = {
> #define CLEANUP xf86freeInputClassList
>
> #define TOKEN_SEP "|"
> +#define TOKEN_KV_SEP ":|"
>
> static void
> add_group_entry(struct xorg_list *head, const char **values)
> @@ -74,6 +77,56 @@ add_group_entry(struct xorg_list *head, const char **values)
> }
> }
>
> +static Bool
> +add_kv_group_entry(struct xorg_list *head, const char **raw,
> + Bool (*is_valid_key)(const char *const key))
> +{
> + char **cur;
> + char *key, *value;
> + int num_token;
> + xf86KVPair *pair;
> + xf86MatchKVGroup *kv_group = NULL;
> +
> + for (num_token = 0; raw[num_token]; num_token++);
please move the ; into the next line to make it more obvious this is a loop
only. also, I'd write this as
while(raw[num_token++])
;
> + /* num_token has to be a multiple of 2 ... key-value pairs */
> + if (num_token % 2)
> + goto error;
you can skip if num_token is 0
check for is_valid_key != NULL here, saves some work later
> +
> + kv_group = malloc(sizeof(*kv_group));
> + if (!kv_group)
> + goto error;
> +
> + kv_group->pairs = calloc(num_token / 2 + 1, sizeof(*kv_group->pairs));
> + if (!kv_group->pairs)
> + goto error;
> +
> + for (cur = (char **) raw, pair = kv_group->pairs; *cur; cur+=2, pair++) {
yikes, please don't cast a const char* to a char*
> + key = *cur;
> + value = *(cur + 1);
> + if (is_valid_key && !(*is_valid_key)(key))
> + goto error;
> +
> + pair->key = key;
> + pair->value = value;
> + }
> +
> + xorg_list_add(&kv_group->entry, head);
> + free(raw);
> +
> + return TRUE;
> +
> +error:
> + for (cur = (char **) raw; *cur; cur++)
> + free((void *) *cur);
> + free(raw);
> +
> + if (kv_group)
> + free(kv_group->pairs);
> + free(kv_group);
> +
> + return FALSE;
again, there's a bit of mixed ownership of the memory here. xstrokenize in
the caller allocates it but then you free it on error but leave it on
success. This should be documented as intentional, just so hunting down
leaks gets easier.
but really, I'd prefer localised memory allocation here, even if it means we
strdup twice.
> +}
> +
> XF86ConfInputClassPtr
> xf86parseInputClassSection(void)
> {
> @@ -90,6 +143,7 @@ xf86parseInputClassSection(void)
> xorg_list_init(&ptr->match_pnpid);
> xorg_list_init(&ptr->match_usbid);
> xorg_list_init(&ptr->match_driver);
> + xorg_list_init(&ptr->match_dmi);
> xorg_list_init(&ptr->match_tag);
> xorg_list_init(&ptr->match_layout);
>
> @@ -167,6 +221,16 @@ xf86parseInputClassSection(void)
> xstrtokenize(xf86_lex_val.str, TOKEN_SEP));
> free(xf86_lex_val.str);
> break;
> + case MATCH_DMI:
> + if (xf86getSubToken(&(ptr->comment)) != STRING)
> + Error(QUOTE_MSG, "MatchDMI");
> + if (!add_kv_group_entry(&ptr->match_dmi,
> + xstrtokenize(xf86_lex_val.str, TOKEN_KV_SEP),
> + xf86DMIIsValidKey))
> + Error("The MatchDMI keyword requires colon separated key "
> + "value pairs. Valid keys can be found in the man-page");
> + free(xf86_lex_val.str);
> + break;
> case MATCH_TAG:
> if (xf86getSubToken(&(ptr->comment)) != STRING)
> Error(QUOTE_MSG, "MatchTag");
> @@ -256,6 +320,8 @@ void
> xf86printInputClassSection(FILE * cf, XF86ConfInputClassPtr ptr)
> {
> const xf86MatchGroup *group;
> + const xf86MatchKVGroup *kv_group;
> + const xf86KVPair *kv_pair;
> const char *const *cur;
>
> while (ptr) {
> @@ -316,6 +382,14 @@ xf86printInputClassSection(FILE * cf, XF86ConfInputClassPtr ptr)
> *cur);
> fprintf(cf, "\"\n");
> }
> + xorg_list_for_each_entry(kv_group, &ptr->match_dmi, entry) {
> + fprintf(cf, "\tMatchDMI \"");
> + for (kv_pair = kv_group->pairs; kv_pair; kv_pair++)
> + fprintf(cf, "%s%s%s%s",
> + kv_pair == kv_group->pairs ? "" : TOKEN_SEP,
> + kv_pair->key, ":", kv_pair->value);
> + fprintf(cf, "\"\n");
> + }
> xorg_list_for_each_entry(group, &ptr->match_tag, entry) {
> fprintf(cf, "\tMatchTag \"");
> for (cur = group->values; *cur; cur++)
> @@ -362,7 +436,9 @@ xf86freeInputClassList(XF86ConfInputClassPtr ptr)
>
> while (ptr) {
> xf86MatchGroup *group, *next;
> + xf86MatchKVGroup *kv_group, *kv_next;
> const char **list;
> + const xf86KVPair *pair;
>
> TestFree(ptr->identifier);
> TestFree(ptr->driver);
> @@ -409,6 +485,15 @@ xf86freeInputClassList(XF86ConfInputClassPtr ptr)
> free((void *) *list);
> free(group);
> }
> + xorg_list_for_each_entry_safe(kv_group, kv_next, &ptr->match_dmi, entry) {
> + xorg_list_del(&kv_group->entry);
> + for (pair = kv_group->pairs; pair; pair++) {
> + free(pair->key);
> + free(pair->value);
> + }
> + free(kv_group->pairs);
> + free(kv_group);
> + }
> xorg_list_for_each_entry_safe(group, next, &ptr->match_tag, entry) {
> xorg_list_del(&group->entry);
> for (list = group->values; *list; list++)
> diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h
> index 83607f2..c140b76 100644
> --- a/hw/xfree86/parser/xf86Parser.h
> +++ b/hw/xfree86/parser/xf86Parser.h
> @@ -303,6 +303,16 @@ typedef struct {
> } xf86MatchGroup;
>
> typedef struct {
> + char *key;
> + char *value;
> +} xf86KVPair;
you could just add a xorg_list to the pair as well and make the code a bit
easier. right now you have a null-terminated list inside an xorg_list, which
seems a bit confusing.
Cheers,
Peter
> +
> +typedef struct {
> + struct xorg_list entry;
> + xf86KVPair *pairs;
> +} xf86MatchKVGroup;
> +
> +typedef struct {
> GenericListRec list;
> char *identifier;
> const char *driver;
> @@ -313,6 +323,7 @@ typedef struct {
> struct xorg_list match_pnpid;
> struct xorg_list match_usbid;
> struct xorg_list match_driver;
> + struct xorg_list match_dmi;
> struct xorg_list match_tag;
> struct xorg_list match_layout;
> xf86TriState is_keyboard;
> diff --git a/hw/xfree86/parser/xf86tokens.h b/hw/xfree86/parser/xf86tokens.h
> index f751b7b..921a5b0 100644
> --- a/hw/xfree86/parser/xf86tokens.h
> +++ b/hw/xfree86/parser/xf86tokens.h
> @@ -278,6 +278,7 @@ typedef enum {
> MATCH_PNPID,
> MATCH_USBID,
> MATCH_DRIVER,
> + MATCH_DMI,
> MATCH_TAG,
> MATCH_LAYOUT,
> MATCH_IS_KEYBOARD,
> --
> 1.8.5.1
>
More information about the xorg-devel
mailing list