linux-kernel - Re: [PATCH] power_supply: Use attribute groups

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: <20100518173501.GI30801@buzzloop.caiaq.de>
Date:	Tue, 18 May 2010 19:35:01 +0200
From:	Daniel Mack <daniel@...aq.de>
To:	Anton Vorontsov <cbouatmailru@...il.com>
Cc:	Kay Sievers <kay.sievers@...y.org>, Greg KH <gregkh@...e.de>,
	linux-kernel@...r.kernel.org,
	David Woodhouse <dwmw2@...radead.org>,
	Alexey Starikovskiy <astarikovskiy@...e.de>,
	Len Brown <len.brown@...el.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Matt Reimer <mreimer@...p.net>,
	Evgeniy Polyakov <zbr@...emap.net>, Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH] power_supply: Use attribute groups

Hi Anton,

On Mon, May 17, 2010 at 11:40:16PM +0400, Anton Vorontsov wrote:
> This fixes a race between power supply device and initial
> attributes creation, plus makes it possible to implement
> writable properties.

Thanks a lot. This works for me, but I have two minor comments below ...

[...]

> diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
> index 5b6e352..f95d934 100644
> --- a/drivers/power/power_supply_sysfs.c
> +++ b/drivers/power/power_supply_sysfs.c
> @@ -41,6 +41,9 @@ static struct device_attribute power_supply_attrs[];
>  static ssize_t power_supply_show_property(struct device *dev,
>  					  struct device_attribute *attr,
>  					  char *buf) {
> +	static char *type_text[] = {
> +		"Battery", "UPS", "Mains", "USB"
> +	};
>  	static char *status_text[] = {
>  		"Unknown", "Charging", "Discharging", "Not charging", "Full"
>  	};
> @@ -58,12 +61,15 @@ static ssize_t power_supply_show_property(struct device *dev,
>  	static char *capacity_level_text[] = {
>  		"Unknown", "Critical", "Low", "Normal", "High", "Full"
>  	};
> -	ssize_t ret;
> +	ssize_t ret = 0;
>  	struct power_supply *psy = dev_get_drvdata(dev);
>  	const ptrdiff_t off = attr - power_supply_attrs;
>  	union power_supply_propval value;
>  
> -	ret = psy->get_property(psy, off, &value);
> +	if (off == POWER_SUPPLY_PROP_TYPE)
> +		value.intval = psy->type;
> +	else
> +		ret = psy->get_property(psy, off, &value);
>  
>  	if (ret < 0) {
>  		if (ret == -ENODATA)
> @@ -85,10 +91,13 @@ static ssize_t power_supply_show_property(struct device *dev,
>  		return sprintf(buf, "%s\n", technology_text[value.intval]);
>  	else if (off == POWER_SUPPLY_PROP_CAPACITY_LEVEL)
>  		return sprintf(buf, "%s\n", capacity_level_text[value.intval]);
> +	else if (off == POWER_SUPPLY_PROP_TYPE)
> +		return sprintf(buf, "%s\n", type_text[value.intval]);
>  	else if (off >= POWER_SUPPLY_PROP_MODEL_NAME)
>  		return sprintf(buf, "%s\n", value.strval);
>  
>  	return sprintf(buf, "%d\n", value.intval);
> +return 0;

This return is superflous :)

And with this approach, we don't need to initialize the .mode field in
the POWER_SUPPLY_ATTR anmore.

I'll then rebase my patches for writeable properties and resend them.

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