Messages in this thread | | | Subject | RE: [PATCH 2/6] PCI Express Advanced Error Reporting Driver | Date | Mon, 14 Mar 2005 10:18:22 -0800 | From | "Nguyen, Tom L" <> |
| |
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/
| |