On Friday, March 11, 2005 11:25 PM Greg KH 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?
Agree, will make changes appropriately. Thanks for your comment.
>> +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 :)
Agree, will make changes appropriately.
>> +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.
Agree, will make changes appropriately.
>> +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.
Agree, will make changes appropriately.
>> +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.
Agree, will make changes appropriately.
Thanks for all comments,
Long
-
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/
