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: <20200503012010.i7wdh4ibsjdzpckr@earth.universe>
Date:   Sun, 3 May 2020 03:20:10 +0200
From:   Sebastian Reichel <sebastian.reichel@...labora.com>
To:     Mathew King <mathewk@...omium.org>
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 3/4] power_supply: Add a macro that maps enum properties
 to text values

Hi,

On Fri, Apr 24, 2020 at 11:35:32AM -0600, Mathew King wrote:
> Reduce the number of touch points to add a new enum property to the
> power_supply class by mapping the array of text values to the device
> attribute descriptor. A new enum property can now added by creating an
> array with the text values named POWER_SUPPLY_${PROPNAME}_TEXT and
> adding POWER_SUPPLY_ENUM_ATTR(${PROPNAME}) to the power_supply_attrs
> array.
> 
> Signed-off-by: Mathew King <mathewk@...omium.org>
> ---

nice cleanup :)

Reviewed-by: Sebastian Reichel <sebastian.reichel@...labora.com>

-- Sebastian

>  drivers/power/supply/power_supply_sysfs.c | 122 +++++++++-------------
>  1 file changed, 49 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 328107589770..fbb05466b9a5 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -26,15 +26,25 @@ struct power_supply_attr {
>  	const char *upper_name;
>  	const char *lower_name;
>  	struct device_attribute dev_attr;
> +	const char * const *text_values;
> +	int text_values_len;
>  };
>  
> -#define POWER_SUPPLY_ATTR(_name)	\
> -[POWER_SUPPLY_PROP_ ## _name] =		\
> -{					\
> -	.prop_name = #_name		\
> +#define _POWER_SUPPLY_ATTR(_name, _text, _len)	\
> +[POWER_SUPPLY_PROP_ ## _name] =			\
> +{						\
> +	.prop_name = #_name,			\
> +	.text_values = _text,			\
> +	.text_values_len = _len,		\
>  }
>  
> -static const char * const power_supply_type_text[] = {
> +#define POWER_SUPPLY_ATTR(_name) _POWER_SUPPLY_ATTR(_name, NULL, 0)
> +#define _POWER_SUPPLY_ENUM_ATTR(_name, _text)	\
> +	_POWER_SUPPLY_ATTR(_name, _text, ARRAY_SIZE(_text))
> +#define POWER_SUPPLY_ENUM_ATTR(_name)	\
> +	_POWER_SUPPLY_ENUM_ATTR(_name, POWER_SUPPLY_ ## _name ## _TEXT)
> +
> +static const char * const POWER_SUPPLY_TYPE_TEXT[] = {
>  	[POWER_SUPPLY_TYPE_UNKNOWN]		= "Unknown",
>  	[POWER_SUPPLY_TYPE_BATTERY]		= "Battery",
>  	[POWER_SUPPLY_TYPE_UPS]			= "UPS",
> @@ -62,7 +72,7 @@ static const char * const POWER_SUPPLY_USB_TYPE_TEXT[] = {
>  	[POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID]	= "BrickID",
>  };
>  
> -static const char * const power_supply_status_text[] = {
> +static const char * const POWER_SUPPLY_STATUS_TEXT[] = {
>  	[POWER_SUPPLY_STATUS_UNKNOWN]		= "Unknown",
>  	[POWER_SUPPLY_STATUS_CHARGING]		= "Charging",
>  	[POWER_SUPPLY_STATUS_DISCHARGING]	= "Discharging",
> @@ -70,7 +80,7 @@ static const char * const power_supply_status_text[] = {
>  	[POWER_SUPPLY_STATUS_FULL]		= "Full",
>  };
>  
> -static const char * const power_supply_charge_type_text[] = {
> +static const char * const POWER_SUPPLY_CHARGE_TYPE_TEXT[] = {
>  	[POWER_SUPPLY_CHARGE_TYPE_UNKNOWN]	= "Unknown",
>  	[POWER_SUPPLY_CHARGE_TYPE_NONE]		= "N/A",
>  	[POWER_SUPPLY_CHARGE_TYPE_TRICKLE]	= "Trickle",
> @@ -80,7 +90,7 @@ static const char * const power_supply_charge_type_text[] = {
>  	[POWER_SUPPLY_CHARGE_TYPE_CUSTOM]	= "Custom",
>  };
>  
> -static const char * const power_supply_health_text[] = {
> +static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
>  	[POWER_SUPPLY_HEALTH_UNKNOWN]		    = "Unknown",
>  	[POWER_SUPPLY_HEALTH_GOOD]		    = "Good",
>  	[POWER_SUPPLY_HEALTH_OVERHEAT]		    = "Overheat",
> @@ -93,7 +103,7 @@ static const char * const power_supply_health_text[] = {
>  	[POWER_SUPPLY_HEALTH_OVERCURRENT]	    = "Over current",
>  };
>  
> -static const char * const power_supply_technology_text[] = {
> +static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
>  	[POWER_SUPPLY_TECHNOLOGY_UNKNOWN]	= "Unknown",
>  	[POWER_SUPPLY_TECHNOLOGY_NiMH]		= "NiMH",
>  	[POWER_SUPPLY_TECHNOLOGY_LION]		= "Li-ion",
> @@ -103,7 +113,7 @@ static const char * const power_supply_technology_text[] = {
>  	[POWER_SUPPLY_TECHNOLOGY_LiMn]		= "LiMn",
>  };
>  
> -static const char * const power_supply_capacity_level_text[] = {
> +static const char * const POWER_SUPPLY_CAPACITY_LEVEL_TEXT[] = {
>  	[POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN]	= "Unknown",
>  	[POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL]	= "Critical",
>  	[POWER_SUPPLY_CAPACITY_LEVEL_LOW]	= "Low",
> @@ -112,7 +122,7 @@ static const char * const power_supply_capacity_level_text[] = {
>  	[POWER_SUPPLY_CAPACITY_LEVEL_FULL]	= "Full",
>  };
>  
> -static const char * const power_supply_scope_text[] = {
> +static const char * const POWER_SUPPLY_SCOPE_TEXT[] = {
>  	[POWER_SUPPLY_SCOPE_UNKNOWN]	= "Unknown",
>  	[POWER_SUPPLY_SCOPE_SYSTEM]	= "System",
>  	[POWER_SUPPLY_SCOPE_DEVICE]	= "Device",
> @@ -120,13 +130,13 @@ static const char * const power_supply_scope_text[] = {
>  
>  static struct power_supply_attr power_supply_attrs[] = {
>  	/* Properties of type `int' */
> -	POWER_SUPPLY_ATTR(STATUS),
> -	POWER_SUPPLY_ATTR(CHARGE_TYPE),
> -	POWER_SUPPLY_ATTR(HEALTH),
> +	POWER_SUPPLY_ENUM_ATTR(STATUS),
> +	POWER_SUPPLY_ENUM_ATTR(CHARGE_TYPE),
> +	POWER_SUPPLY_ENUM_ATTR(HEALTH),
>  	POWER_SUPPLY_ATTR(PRESENT),
>  	POWER_SUPPLY_ATTR(ONLINE),
>  	POWER_SUPPLY_ATTR(AUTHENTIC),
> -	POWER_SUPPLY_ATTR(TECHNOLOGY),
> +	POWER_SUPPLY_ENUM_ATTR(TECHNOLOGY),
>  	POWER_SUPPLY_ATTR(CYCLE_COUNT),
>  	POWER_SUPPLY_ATTR(VOLTAGE_MAX),
>  	POWER_SUPPLY_ATTR(VOLTAGE_MIN),
> @@ -169,7 +179,7 @@ static struct power_supply_attr power_supply_attrs[] = {
>  	POWER_SUPPLY_ATTR(CAPACITY),
>  	POWER_SUPPLY_ATTR(CAPACITY_ALERT_MIN),
>  	POWER_SUPPLY_ATTR(CAPACITY_ALERT_MAX),
> -	POWER_SUPPLY_ATTR(CAPACITY_LEVEL),
> +	POWER_SUPPLY_ENUM_ATTR(CAPACITY_LEVEL),
>  	POWER_SUPPLY_ATTR(TEMP),
>  	POWER_SUPPLY_ATTR(TEMP_MAX),
>  	POWER_SUPPLY_ATTR(TEMP_MIN),
> @@ -182,9 +192,9 @@ static struct power_supply_attr power_supply_attrs[] = {
>  	POWER_SUPPLY_ATTR(TIME_TO_EMPTY_AVG),
>  	POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
>  	POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
> -	POWER_SUPPLY_ATTR(TYPE),
> +	POWER_SUPPLY_ENUM_ATTR(TYPE),
>  	POWER_SUPPLY_ATTR(USB_TYPE),
> -	POWER_SUPPLY_ATTR(SCOPE),
> +	POWER_SUPPLY_ENUM_ATTR(SCOPE),
>  	POWER_SUPPLY_ATTR(PRECHARGE_CURRENT),
>  	POWER_SUPPLY_ATTR(CHARGE_TERM_CURRENT),
>  	POWER_SUPPLY_ATTR(CALIBRATE),
> @@ -197,10 +207,14 @@ static struct power_supply_attr power_supply_attrs[] = {
>  static struct attribute *
>  __power_supply_attrs[ARRAY_SIZE(power_supply_attrs) + 1];
>  
> +static struct power_supply_attr *to_ps_attr(struct device_attribute *attr)
> +{
> +	return container_of(attr, struct power_supply_attr, dev_attr);
> +}
> +
>  static enum power_supply_property dev_attr_psp(struct device_attribute *attr)
>  {
> -	return container_of(attr, struct power_supply_attr, dev_attr) -
> -		power_supply_attrs;
> +	return  to_ps_attr(attr) - power_supply_attrs;
>  }
>  
>  static ssize_t power_supply_show_usb_type(struct device *dev,
> @@ -219,11 +233,11 @@ static ssize_t power_supply_show_usb_type(struct device *dev,
>  
>  		if (value->intval == usb_type) {
>  			count += sprintf(buf + count, "[%s] ",
> -					 power_supply_usb_type_text[usb_type]);
> +					 POWER_SUPPLY_USB_TYPE_TEXT[usb_type]);
>  			match = true;
>  		} else {
>  			count += sprintf(buf + count, "%s ",
> -					 power_supply_usb_type_text[usb_type]);
> +					 POWER_SUPPLY_USB_TYPE_TEXT[usb_type]);
>  		}
>  	}
>  
> @@ -243,6 +257,7 @@ static ssize_t power_supply_show_property(struct device *dev,
>  					  char *buf) {
>  	ssize_t ret;
>  	struct power_supply *psy = dev_get_drvdata(dev);
> +	struct power_supply_attr *ps_attr = to_ps_attr(attr);
>  	enum power_supply_property psp = dev_attr_psp(attr);
>  	union power_supply_propval value;
>  
> @@ -263,39 +278,16 @@ static ssize_t power_supply_show_property(struct device *dev,
>  		}
>  	}
>  
> +	if (ps_attr->text_values_len > 0 &&
> +	    value.intval < ps_attr->text_values_len && value.intval >= 0) {
> +		return sprintf(buf, "%s\n", ps_attr->text_values[value.intval]);
> +	}
> +
>  	switch (psp) {
> -	case POWER_SUPPLY_PROP_STATUS:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_status_text[value.intval]);
> -		break;
> -	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_charge_type_text[value.intval]);
> -		break;
> -	case POWER_SUPPLY_PROP_HEALTH:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_health_text[value.intval]);
> -		break;
> -	case POWER_SUPPLY_PROP_TECHNOLOGY:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_technology_text[value.intval]);
> -		break;
> -	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_capacity_level_text[value.intval]);
> -		break;
> -	case POWER_SUPPLY_PROP_TYPE:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_type_text[value.intval]);
> -		break;
>  	case POWER_SUPPLY_PROP_USB_TYPE:
>  		ret = power_supply_show_usb_type(dev, psy->desc->usb_types,
> -						 psy->desc->num_usb_types,
> -						 &value, buf);
> -		break;
> -	case POWER_SUPPLY_PROP_SCOPE:
> -		ret = sprintf(buf, "%s\n",
> -			      power_supply_scope_text[value.intval]);
> +						psy->desc->num_usb_types,
> +						&value, buf);
>  		break;
>  	case POWER_SUPPLY_PROP_MODEL_NAME ... POWER_SUPPLY_PROP_SERIAL_NUMBER:
>  		ret = sprintf(buf, "%s\n", value.strval);
> @@ -312,30 +304,14 @@ static ssize_t power_supply_store_property(struct device *dev,
>  					   const char *buf, size_t count) {
>  	ssize_t ret;
>  	struct power_supply *psy = dev_get_drvdata(dev);
> +	struct power_supply_attr *ps_attr = to_ps_attr(attr);
>  	enum power_supply_property psp = dev_attr_psp(attr);
>  	union power_supply_propval value;
>  
> -	switch (psp) {
> -	case POWER_SUPPLY_PROP_STATUS:
> -		ret = sysfs_match_string(power_supply_status_text, buf);
> -		break;
> -	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> -		ret = sysfs_match_string(power_supply_charge_type_text, buf);
> -		break;
> -	case POWER_SUPPLY_PROP_HEALTH:
> -		ret = sysfs_match_string(power_supply_health_text, buf);
> -		break;
> -	case POWER_SUPPLY_PROP_TECHNOLOGY:
> -		ret = sysfs_match_string(power_supply_technology_text, buf);
> -		break;
> -	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
> -		ret = sysfs_match_string(power_supply_capacity_level_text, buf);
> -		break;
> -	case POWER_SUPPLY_PROP_SCOPE:
> -		ret = sysfs_match_string(power_supply_scope_text, buf);
> -		break;
> -	default:
> -		ret = -EINVAL;
> +	ret = -EINVAL;
> +	if (ps_attr->text_values_len > 0) {
> +		ret = __sysfs_match_string(ps_attr->text_values,
> +					   ps_attr->text_values_len, buf);
>  	}
>  
>  	/*
> -- 
> 2.26.2.303.gf8c07b1a785-goog
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ