Hi Mark,
On Sun, 16 Dec 2007 13:59:02 -0500, Mark M. Hoffman wrote:
> Hi Jean:
>
> * Jean Delvare <khali at linux-fr.org> [2007-12-10 14:30:57 +0100]:
> > On Sun, 2 Dec 2007 21:42:39 +0100, Jean Delvare wrote:
> > > Well, I have no reason to believe that anything is wrong with this
> > > patch, as all my testing has been successful. Feel free to apply it to
> > > Rawhide if you want. I would like to commit the change soon, the only
> > > thing I am waiting for at the moment is Mark's ack, as we had been
> > > discussing the matter before, he might want to comment on the approach
> > > I took in my patch.
> >
> > Patch is committed now.
>
> I've finally finished reviewing this. I only have one minor change to
> suggest, which IMHO makes the code a little more readable at very little
> expense in cycles and no expense in size.
>
> Index: lib/sysfs.c
> ===================================================================
> --- lib/sysfs.c (revision 5072)
> +++ lib/sysfs.c (working copy)
> @@ -73,7 +73,7 @@
> * local errors, or a negative error value if any call fails.
> */
> static int sysfs_foreach_classdev(const char *class_name,
> - int (*func)(char *, const char*))
> + int (*func)(const char *, const char *))
> {
> char path[NAME_MAX];
> int path_off, ret;
> @@ -105,7 +105,7 @@
> * local errors, or a negative error value if any call fails.
> */
> static int sysfs_foreach_busdev(const char *bus_type,
> - int (*func)(char *, const char*))
> + int (*func)(const char *, const char *))
> {
> char path[NAME_MAX];
> int path_off, ret;
> @@ -455,7 +455,7 @@
> }
>
> /* returns: 0 if successful, !0 otherwise */
> -static int sensors_read_one_sysfs_chip(char *dev_path, const char *dev_name)
> +static int sensors_read_one_sysfs_chip(const char *dev_path, const char *dev_name)
> {
> int domain, bus, slot, fn;
> int err = -SENSORS_ERR_KERNEL;
> @@ -539,20 +539,20 @@
> return 0;
> }
>
> -static int sensors_add_hwmon_device(char *path, const char *classdev)
> +static int sensors_add_hwmon_device(const char *path, const char *classdev)
> {
> + char linkpath[NAME_MAX];
> char device[NAME_MAX];
> - int path_off = strlen(path);
> int dev_len;
> (void)classdev; /* hide warning */
>
> - snprintf(path + path_off, NAME_MAX - path_off, "/device");
> - dev_len = readlink(path, device, NAME_MAX - 1);
> + snprintf(linkpath, NAME_MAX, "%s/device", path);
> + dev_len = readlink(linkpath, device, NAME_MAX - 1);
> if (dev_len < 0)
> return -SENSORS_ERR_KERNEL;
> device[dev_len] = '\0';
>
> - return sensors_read_one_sysfs_chip(path, strrchr(device, '/') + 1);
> + return sensors_read_one_sysfs_chip(linkpath, strrchr(device, '/') + 1);
> }
>
> /* returns 0 if successful, !0 otherwise */
> @@ -572,7 +572,7 @@
> }
>
> /* returns 0 if successful, !0 otherwise */
> -static int sensors_add_i2c_bus(char *path, const char *classdev)
> +static int sensors_add_i2c_bus(const char *path, const char *classdev)
> {
> sensors_bus entry;
>
I'm totally fine with this patch, I agree that it's somewhat cleaner
and the additional cost is negligible. Feel free to apply it to SVN.
Thanks for the review,
--
Jean Delvare
