On 2007.05.08 02:54:29 +0200, Lennart Poettering wrote:
> From: Lennart Poettering <[EMAIL PROTECTED]>
> +#define DEFINE_DMI_ATTR_WITH_SHOW(_name, _mode, _field)
> \
Hm, too many tabs here? Wraps for me with tabsize=8.
> +static ssize_t sys_dmi_##_name##_show(struct device *dev,
> \
> + struct device_attribute *attr, char
> *page) \
> +{
> \
> + ssize_t len;
> \
> + len = snprintf(page, PAGE_SIZE, "%s\n", dmi_get_system_info(_field));
> \
> + page[PAGE_SIZE-2] = '\n';
> \
> + page[PAGE_SIZE-1] = 0;
> \
> + return min_t(ssize_t, len, PAGE_SIZE);
> \
> +}
> \
If you return PAGE_SIZE here, that includes the trailing 0, while len
does not. Seems not to cause any problems, but scnprintf already handles
that, so how about this?
ssize_t len;
len = scnprintf(page, PAGE_SIZE, "%s\n", dmi_get_system_info(_field));
page[len - 1] = '\n';
return len;
> +DEFINE_DMI_ATTR(_name, _mode, sys_dmi_##_name##_show);
> +
> +DEFINE_DMI_ATTR_WITH_SHOW(bios_vendor, 0444, DMI_BIOS_VENDOR);
> +DEFINE_DMI_ATTR_WITH_SHOW(bios_version, 0444, DMI_BIOS_VERSION);
> +DEFINE_DMI_ATTR_WITH_SHOW(bios_date, 0444, DMI_BIOS_DATE);
> +DEFINE_DMI_ATTR_WITH_SHOW(sys_vendor, 0444, DMI_SYS_VENDOR);
> +DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
> +DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
> +DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
> +DEFINE_DMI_ATTR_WITH_SHOW(product_uuid, 0400, DMI_PRODUCT_UUID);
> +DEFINE_DMI_ATTR_WITH_SHOW(board_vendor, 0444, DMI_BOARD_VENDOR);
> +DEFINE_DMI_ATTR_WITH_SHOW(board_name, 0444, DMI_BOARD_NAME);
> +DEFINE_DMI_ATTR_WITH_SHOW(board_version, 0444, DMI_BOARD_VERSION);
> +DEFINE_DMI_ATTR_WITH_SHOW(board_serial, 0400, DMI_BOARD_SERIAL);
> +DEFINE_DMI_ATTR_WITH_SHOW(board_asset_tag, 0444, DMI_BOARD_ASSET_TAG);
> +DEFINE_DMI_ATTR_WITH_SHOW(chassis_vendor, 0444, DMI_CHASSIS_VENDOR);
> +DEFINE_DMI_ATTR_WITH_SHOW(chassis_type, 0444, DMI_CHASSIS_TYPE);
> +DEFINE_DMI_ATTR_WITH_SHOW(chassis_version, 0444, DMI_CHASSIS_VERSION);
> +DEFINE_DMI_ATTR_WITH_SHOW(chassis_serial, 0400, DMI_CHASSIS_SERIAL);
> +DEFINE_DMI_ATTR_WITH_SHOW(chassis_asset_tag, 0444, DMI_CHASSIS_ASSET_TAG);
Mixed tab/space alignment.
> +static void ascii_filter(char *d, const char *s)
> +{
> + /* Filter out characters we don't want to see in the modalias string */
> + for (; *s; s++)
> + if (*s > ' ' && *s < 127 && *s != ':')
> + *(d++) = *s;
> +
> + *d = 0;
> +}
> +
> +static ssize_t get_modalias(char *buffer, size_t buffer_size)
> +{
> + struct mafield {
> + const char *prefix;
> + int field;
> + };
> +
Trailing whitespace (also in a few more places)
> + static const struct mafield fields[] = {
> + { "bvn", DMI_BIOS_VENDOR },
> + { "bvr", DMI_BIOS_VERSION },
> + { "bd", DMI_BIOS_DATE },
> + { "svn", DMI_SYS_VENDOR },
> + { "pn", DMI_PRODUCT_NAME },
> + { "pvr", DMI_PRODUCT_VERSION },
> + { "rvn", DMI_BOARD_VENDOR },
> + { "rn", DMI_BOARD_NAME },
> + { "rvr", DMI_BOARD_VERSION },
> + { "cvn", DMI_CHASSIS_VENDOR },
> + { "ct", DMI_CHASSIS_TYPE },
> + { "cvr", DMI_CHASSIS_VERSION },
> + { NULL, DMI_NONE }
Mixed tab/space alignment.
> + };
> +
> + ssize_t l, left;
> + char *p;
> + const struct mafield *f;
> +
> + strcpy(buffer, "dmi");
> + p = buffer + 3; left = buffer_size-5;
> +
> + for (f = fields; f->prefix && left > 0; f++) {
> + const char *c;
> + char *t;
> +
> + c = dmi_get_system_info(f->field);
> + if (!c)
> + continue;
> +
> + t = kmalloc(strlen(c), GFP_KERNEL);
> + if (!t)
> + break;
> + ascii_filter(t, c);
> + l = snprintf(p, left, ":%s%s", f->prefix, t);
Looks like a candidate for scnprintf, too, avoids the l > left
comparison below.
> + kfree(t);
> +
> + if (l > left)
> + l = left;
> +
> + p += l;
> + left -= l;
> + }
> +
> + snprintf(p, left, ":");
I assume that the colon is strictly required as -5 is used above to
calculate how much space is left. But "left" might be 0 if the above
snprintf had to truncate a string, thus the colon would not be written,
although there's space left. Fails for left == 1, too.
Björn
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
