lkml.org 
[lkml]   [2005]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRE: [PATCH 2/6] PCI Express Advanced Error Reporting Driver
Date
From
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:11    [W:0.033 / U:2.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site