[PATCH 4/5] xfree86/linux: implement xf86DMIInit

[Peter Hutterer]

On Sat, Dec 07, 2013 at 04:48:49PM +0100, Daniel Martin wrote:
> Read DMI identifiers from sysfs (/sys/devices/virtual/dmi/id).
> 
> Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> ---
>  hw/xfree86/os-support/linux/Makefile.am |   2 +-
>  hw/xfree86/os-support/linux/lnx_dmi.c   | 136 ++++++++++++++++++++++++++++++++
>  2 files changed, 137 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xfree86/os-support/linux/Makefile.am b/hw/xfree86/os-support/linux/Makefile.am
> index 2918c38..4fe38a8 100644
> --- a/hw/xfree86/os-support/linux/Makefile.am
> +++ b/hw/xfree86/os-support/linux/Makefile.am
> @@ -28,12 +28,12 @@ liblinux_la_SOURCES = \
>  	$(srcdir)/../shared/posix_tty.c \
>  	$(srcdir)/../shared/sigio.c \
>  	$(srcdir)/../shared/vidmem.c \
> -	$(srcdir)/../stub/stub_dmi.c \
>  	$(ACPI_SRCS) \
>  	$(APM_SRCS) \
>  	$(PLATFORM_PCI_SUPPORT) \
>  	lnx_agp.c \
>  	lnx_bell.c \
> +	lnx_dmi.c \
>  	lnx_init.c \
>  	lnx_kmod.c \
>  	lnx_platform.c \
> diff --git a/hw/xfree86/os-support/linux/lnx_dmi.c b/hw/xfree86/os-support/linux/lnx_dmi.c
> new file mode 100644
> index 0000000..cf98dfb
> --- /dev/null
> +++ b/hw/xfree86/os-support/linux/lnx_dmi.c
> @@ -0,0 +1,136 @@
> +#ifdef HAVE_XORG_CONFIG_H
> +#include <xorg-config.h>
> +#endif
> +
> +#include <dirent.h>
> +
> +#include "parser/xf86Parser.h"
> +#include "xf86.h"
> +#include "xf86_OSlib.h"
> +
> +
> +static const char SYSFS_DMI_PATH[] = "/sys/devices/virtual/dmi/id";
> +
> +struct dname_to_key {
> +    const char *const d_name;
> +    const char *const key;
> +}
> +/* map filenames in SYSFS_DMI_PATH to a key */
> +static const SYSFS_DMI_ENTRY_MAP[] = {
> +    {"bios_date", "BiosDate"},
> +    {"bios_vendor", "BiosVendor"},
> +    {"bios_version", "BiosVersion"},
> +    {"board_name", "BoardName"},
> +    {"board_vendor", "BoardVendor"},
> +    {"board_version", "BoardVersion"},
> +    {"chassis_type", "ChassisType"},
> +    {"chassis_vendor", "ChassisVendor"},
> +    {"chassis_version", "ChassisVersion"},
> +    {"product_name", "ProductName"},
> +    {"product_version", "ProductVersion"},
> +    {"sys_vendor", "SystemVendor"},
> +    {NULL, NULL}
> +};
> +
> +/*
> + * Get the key for a filename.
> + */
> +static const char *const
> +get_key(const char *const d_name)
> +{
> +    const struct dname_to_key *cur;
> +
> +    for (cur = &SYSFS_DMI_ENTRY_MAP[0]; cur->d_name; cur++)

tbh, the capitalised names here read a bit odd to me, just lowercase them.

> +        if (strcmp(cur->d_name, d_name) == 0)
> +            return cur->key;
> +
> +    return NULL;
> +}
> +
> +/*
> + * Get the DMI id value from a file in SYSFS_DMI_PATH. The content of the
> + * file will be written to 'value'.
> + *
> + * Returns FALSE if the file couldn't be read or the string length of the
> + * value is zeor.

typo

> + */
> +static Bool
> +get_value(const char *const d_name, char *value, size_t value_sz)
> +{
> +    int fd;
> +    char fn[PATH_MAX];
> +    size_t num_read;
> +
> +    snprintf(fn, sizeof(fn), "%s/%s", SYSFS_DMI_PATH, d_name);
> +    fd = open(fn, O_RDONLY, 0);
> +    if (fd < 0)
> +        return FALSE;
> +
> +    num_read = read(fd, value, value_sz);
> +    close(fd);
> +
> +    return xrtrim(value, num_read) > 0 ? TRUE : FALSE;
> +}
> +
> +/*
> + * Filter out uninteresting files.
> + */
> +static int
> +scandir_filter(const struct dirent *entry)
> +{
> +    /* skip none regular files */

typo, "non-regular"

> +    if (entry->d_type != DT_REG)
> +        return 0;
> +
> +    /* skip files without a mapping from filename to key */
> +    return get_key(entry->d_name) ? 1 : 0;
> +}
> +
> +void
> +xf86DMIInit(void)
> +{
> +    int i, num_entries;
> +    struct dirent **entries;
> +
> +    char buf[PATH_MAX];
> +
> +    Bool error = FALSE;
> +
> +    num_entries = scandir(SYSFS_DMI_PATH, &entries,
> +                          scandir_filter, alphasort);
> +    if (num_entries < 0) {
> +        xf86MsgVerb(X_ERROR, 3, "Can't read DMI identifiers (%s) (%s)\n",
> +                    SYSFS_DMI_PATH, strerror(errno));
> +        return;
> +    }
> +
> +    for (i = 0; i < num_entries; i++) {
> +        const char *key;
> +        char *value;
> +
> +        key = get_key(entries[i]->d_name);
> +        if (!key)
> +            continue;
> +
> +        if (!get_value(entries[i]->d_name, &buf[0], sizeof(buf)))

passing &buf[0] is a bit odd in this context, just pass buf?

also, if you let get_value return a strdup() string or NULL on failure you
can skip the extra error check below.

> +            /* Couldn't open/read the file or strlen of the buf is zero. */
> +            continue;
> +
> +        value = strdup(buf);
> +        if (!value) {
> +            error = TRUE;
> +            break;
> +        }
> +
> +        xf86DMIAdd(key, value);

see comment on other patch, but if this can fail now (after removing the
assert) you'll need an extra cleanup path here.

other than that, series looks good and it's certainly a feature we'll need,
especially for the t440s but we could've used this in the past a few times
already (and every time for synaptics configuration :(

Cheers,
   Peter

> +    }
> +
> +    /* always cleanup entries */
> +    for (i = 0; i < num_entries; i++)
> +        free(entries[i]);
> +    free(entries);
> +
> +    if (error) {
> +        xf86DMICleanup();
> +    }
> +}
> -- 
> 1.8.4.2