linux-kernel - Re: [PATCH 1/4] extcon: Modify the device name as extcon[X] for sysfs

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: <20150428141714.GK3480@opensource.wolfsonmicro.com>
Date:	Tue, 28 Apr 2015 15:17:14 +0100
From:	Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
To:	Chanwoo Choi <cw00.choi@...sung.com>
Cc:	linux-kernel@...r.kernel.org, myungjoo.ham@...sung.com,
	kyungmin.park@...sung.com
Subject: Re: [PATCH 1/4] extcon: Modify the device name as extcon[X] for
	sysfs

On Mon, Apr 27, 2015 at 09:13:35PM +0900, Chanwoo Choi wrote:
> This patch modify the device name as extcon[X] for sysfs by using the 'extcon'
> prefix word instead of separate device name. On user-space aspect, user would
> find the some extcon drvier with extcon[X] pattern. So, this patch modify the
> device name as following:
> - /sys/class/extcon/[device name] -> /sys/class/extcon/extcon[X]
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
> ---
>  drivers/extcon/extcon.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 4c9f165..1a93229 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -163,7 +163,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
>  			return ret;
>  	}
>  
> -	return sprintf(buf, "%s\n", dev_name(&edev->dev));
> +	return sprintf(buf, "%s\n", edev->name);
>  }
>  static DEVICE_ATTR_RO(name);
>  
> @@ -701,6 +701,7 @@ EXPORT_SYMBOL_GPL(devm_extcon_dev_free);
>  int extcon_dev_register(struct extcon_dev *edev)
>  {
>  	int ret, index = 0;
> +	static atomic_t edev_no = ATOMIC_INIT(-1);
>  
>  	if (!extcon_class) {
>  		ret = create_extcon_class();
> @@ -725,13 +726,14 @@ int extcon_dev_register(struct extcon_dev *edev)
>  	edev->dev.class = extcon_class;
>  	edev->dev.release = extcon_dev_release;
>  
> -	edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
> +	edev->name = dev_name(edev->dev.parent);
>  	if (IS_ERR_OR_NULL(edev->name)) {
>  		dev_err(&edev->dev,
>  			"extcon device name is null\n");
>  		return -EINVAL;
>  	}
> -	dev_set_name(&edev->dev, "%s", edev->name);
> +	dev_set_name(&edev->dev, "extcon%lu",
> +			(unsigned long)atomic_inc_return(&edev_no));
>  
>  	if (edev->max_supported) {
>  		char buf[10];
> -- 
> 1.8.5.5
> 

I am not quite sure I see the advantage of this. Why is naming
the node extcon[X] better than the old system? Seems like the
older more descriptive names are better on the face of it, unless
there is some problem with them I am missing.

It also looks problematic, it changes the ABI for the Arizona
extcon driver for a start, admittedly there arn't many users for
the extcon driver at the moment but still is far from ideal to
break the ABI. Secondly, the order of the extcon[X]'s will not be
guaranteed, add some new hardware to the system probe order
changes and now what was extcon0 is extcon1. So it somewhat
complicates the user-space code as it now has to work out if the
device is the one it wants whereas before it could just hard-code
the name.

Thanks,
Charles
--
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/