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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080515095246.3b6a0d1d@gondolin.boeblingen.de.ibm.com>
Date:	Thu, 15 May 2008 09:52:46 +0200
From:	Cornelia Huck <cornelia.huck@...ibm.com>
To:	Stephen Hemminger <shemminger@...tta.com>
Cc:	Greg KH <greg@...ah.com>, David Miller <davem@...emloft.net>,
	fubar@...ibm.com, netdev@...r.kernel.org,
	bonding-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] sysfs: remove error messages for -EEXIST case

On Wed, 14 May 2008 18:16:03 -0700,
Stephen Hemminger <shemminger@...tta.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/