On Fri, Mar 11, 2005 at 04:13:33PM -0800, long wrote:
> +static ssize_t aer_sysfs_consume_show(struct device_driver *dev, char *buf)
> +{
> + return aer_fsprint_record(buf);
> +}
> +
> +static ssize_t aer_sysfs_status_show(struct device_driver *dev, char *buf)
> +{
> + return aer_fsprint_devices(buf);
> +}
> +
Why call wrapper functions that only do one thing? Why have this extra
layer of indirection that is not needed from what I can tell?
> +static ssize_t aer_sysfs_verbose_show(struct device_driver *dev, char *buf)
> +{
> + return sprintf(buf, "Verbose display set to %d\n",
> + aer_get_verbose());
> +}
Just echo the value, don't print out pretty strings :)
> +static ssize_t aer_sysfs_verbose_store(struct device_driver *drv,
> + const char *buf, size_t count)
> +{
> + aer_set_verbose(buf[0] - 0x30);
> + return count;
> +}
Oh, that's a problem waiting to happen... Please validate the user
provided value before acting on it.
> +static ssize_t aer_sysfs_auto_show(struct device_driver *dev, char *buf)
> +{
> + return sprintf(buf, "Automatic reporting is %s\n",
> + (aer_get_auto_mode()) ? "on" : "off");
> +}
Again, just print on/off.
> +static ssize_t aer_sysfs_auto_store(struct device_driver *drv,
> + const char *buf, size_t count)
> +{
> + aer_set_auto_mode(buf[0] - 0x30);
> + return count;
> +}
Also validate this.
thanks,
greg k-h
-
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/
