Re: [PATCH 2/3] sysfs: remove error messages for -EEXIST case

From: Cornelia Huck
Date: Thu May 15 2008 - 03:53:38 EST


On Wed, 14 May 2008 18:16:03 -0700,
Stephen Hemminger <shemminger@xxxxxxxxxx> wrote:

> --- a/drivers/base/core.c 2008-05-14 17:56:36.000000000 -0700
> +++ b/drivers/base/core.c 2008-05-14 17:56:40.000000000 -0700
> @@ -1218,13 +1218,11 @@ int device_rename(struct device *dev, ch
> }
> #else
> if (dev->class) {
> - sysfs_remove_link(&dev->class->subsys.kobj, old_device_name);
> error = sysfs_create_link(&dev->class->subsys.kobj, &dev->kobj,
> dev->bus_id);
> - if (error) {
> - dev_err(dev, "%s: sysfs_create_symlink failed (%d)\n",
> - __func__, error);
> - }
> + if (error)
> + goto out;
> + sysfs_remove_link(&dev->class->subsys.kobj, old_device_name);
> }
> #endif
>

This looks reasonable...

> --- a/fs/sysfs/dir.c 2008-05-14 17:56:36.000000000 -0700
> +++ b/fs/sysfs/dir.c 2008-05-14 17:56:40.000000000 -0700
> @@ -419,12 +419,8 @@ void sysfs_addrm_start(struct sysfs_addr
> */
> int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
> {
> - if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) {
> - printk(KERN_WARNING "sysfs: duplicate filename '%s' "
> - "can not be created\n", sd->s_name);
> - WARN_ON(1);
> + if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
> return -EEXIST;
> - }
>
> sd->s_parent = sysfs_get(acxt->parent_sd);

...but this will cause many useful warnings to disappear.

What to do here?

- Rely on people checking all __must_check stuff and printing a warning
when desired. Not everyone seems to like that (see, for example,
http://marc.info/?l=linux-kernel&m=121012176111154&w=2).
- Put the burden unto the callers of sysfs_add_one() (or maybe even
further up). They should hopefully know whether -EEXIST is "might
happen" or "argh, we've messed up" (and print a warning in the latter
case so that it gets reported).
- Make this a debugging thing. Unfortunately then we won't neccessarily
get reports when things are busted.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/