Messages in this thread | | | Date | Mon, 14 Apr 2008 21:46:21 -0700 | From | "Dan Williams" <> | Subject | Re: [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor |
| |
On Mon, Apr 14, 2008 at 8:13 PM, SL Baur <steve@xemacs.org> wrote: > On 4/14/08, Dan Williams <dan.j.williams@intel.com> wrote: > > > This is the second revision of the patch originally posted here: > > http://marc.info/?l=linux-kernel&m=120795638915272&w=2 > > > > * Fixed up ENOMEM handling in devices_init() > > * Added a short blurb in Documentation/filesystems/sysfs.txt > > > > Documentation/filesystems/sysfs.txt | 6 +++++ > > drivers/base/core.c | 46 ++++++++++++++++++++++++++++++++++- > > 2 files changed, 51 insertions(+), 1 deletions(-) > > I've looked this patch over and I have some comments. The logic looks > correct, but there are two ugly lines. > > > > @@ -775,6 +783,7 @@ int device_add(struct device *dev) > > struct device *parent = NULL; > > struct class_interface *class_intf; > > int error; > > + char devt_str[25]; > > > > @@ -925,12 +944,16 @@ void device_del(struct device *dev) > > { > > struct device *parent = dev->parent; > > struct class_interface *class_intf; > > + char devt_str[25]; > > May I ask why `25'? The only other user of format_dev_t that I could find > in a quick grep is md (device-mapper) and they used a hardcoded `15' there. > The real problem is format_dev_t and print_dev_t. > > If the only other user of those macros which want to be C inlines with > buffer size parameters is md, perhaps now would be a good time to clean > them up before adding more users? > > Otherwise, the logic looks O.K. >
Thanks for looking it over. To be honest I was iffy about those buffer sizes as well, and looking closer they are overkill. Although, they are smaller than the 32-byte buffer in bsg.c :-). 13-bytes is all that is needed given a 12-bit major and 20-bit minor. Care to send a patch to fix up format_dev_t and print_dev_t?
> Add my > Reviewed-by: SL Baur <steve@xemacs.org> > if that is appropriate. >
Yes, appropriate.
Thanks, Dan
| |