[PATCH] platform/x86: hp-bioscfg: consolidate common package-element parsing

[PATCH] platform/x86: hp-bioscfg: consolidate common package-element parsing

[Muhammad Bilal]

hp_populate_string_elements_from_package(),
hp_populate_integer_elements_from_package(),
hp_populate_enumeration_elements_from_package(),
hp_populate_ordered_list_elements_from_package(), and
hp_populate_password_elements_from_package() each parse the same
PATH..SECURITY_LEVEL range into their own struct common_data
sub-object, copy-pasted five times. Factor it into
hp_get_common_data_from_package(), called from all five, mirroring
hp_get_common_data_from_buffer()'s existing split for the buffer
side.

The five PREREQUISITES loops weren't identical: on a bad
prerequisite string, string/integer/ordered_list continue,
enumeration returns -EINVAL, password breaks. The shared helper
continues, matching the majority and
hp_get_common_data_from_buffer()'s own handling of the equivalent
failure. This changes enumeration (no longer aborts the package)
and password (no longer leaves later prerequisites unfilled).

The expected_*_types[eloc] != obj[elem].type check above this
switch has the same split -- enumeration warns and continues, the
rest pr_err() and return -EIO -- left alone here; consolidating it
needs each type's expected_*_types[] table passed into a shared
checker, not just struct common_data.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c  | 97 +++++++++++++++++++
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h  |  3 +
 .../x86/hp/hp-bioscfg/enum-attributes.c       | 57 +----------
 .../x86/hp/hp-bioscfg/int-attributes.c        | 57 +----------
 .../x86/hp/hp-bioscfg/order-list-attributes.c | 58 +----------
 .../x86/hp/hp-bioscfg/passwdobj-attributes.c  | 57 +----------
 .../x86/hp/hp-bioscfg/string-attributes.c     | 58 +----------
 7 files changed, 125 insertions(+), 262 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 309634c1cc20..22c198680903 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -484,6 +484,103 @@ int hp_convert_hexstr_to_str(const char *input, u32 input_len, char **str, int *
 	return ret;
 }
 
+/**
+ * hp_get_common_data_from_package() - Parse the package elements shared
+ * by every attribute type (PATH..SECURITY_LEVEL)
+ * @obj: ACPI object package for this attribute instance
+ * @obj_count: Number of elements in @obj
+ * @elem: Current position in @obj; advanced in place when PREREQUISITES
+ *	  consumes additional array elements
+ * @eloc: Current common-element locator; advanced in place when
+ *	  PREREQUISITES_SIZE is zero, since PREREQUISITES is then omitted
+ *	  by BIOS
+ * @int_value: Integer value already decoded by the caller for *@eloc,
+ *	       when applicable
+ * @str_value: String value already decoded by the caller for *@eloc, when
+ *	       applicable; reused and reset to NULL while walking
+ *	       PREREQUISITES, same as the caller does with its own copy
+ * @common: Destination common_data struct for this attribute instance
+ *
+ * Called by each type's own hp_populate_*_elements_from_package()
+ * instead of duplicating this parsing per type, since every BIOS
+ * attribute package uses the same layout for these elements (see
+ * struct common_data).
+ *
+ * Return: 0 on success, or -EINVAL if @obj is too small for the
+ * declared PREREQUISITES_SIZE.
+ */
+int hp_get_common_data_from_package(union acpi_object *obj, int obj_count,
+				    int *elem, int *eloc, u32 int_value,
+				    char **str_value, struct common_data *common)
+{
+	int value_len;
+	int reqs;
+	int size;
+
+	switch (*eloc) {
+	case PATH:
+		strscpy(common->path, *str_value);
+		break;
+	case IS_READONLY:
+		common->is_readonly = int_value;
+		break;
+	case DISPLAY_IN_UI:
+		common->display_in_ui = int_value;
+		break;
+	case REQUIRES_PHYSICAL_PRESENCE:
+		common->requires_physical_presence = int_value;
+		break;
+	case SEQUENCE:
+		common->sequence = int_value;
+		break;
+	case PREREQUISITES_SIZE:
+		if (int_value > MAX_PREREQUISITES_SIZE) {
+			pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+			int_value = MAX_PREREQUISITES_SIZE;
+		}
+		common->prerequisites_size = int_value;
+
+		/*
+		 * This step is needed to keep the expected
+		 * element list pointing to the right obj[elem].type
+		 * when the size is zero. PREREQUISITES
+		 * object is omitted by BIOS when the size is
+		 * zero.
+		 */
+		if (common->prerequisites_size == 0)
+			(*eloc)++;
+		break;
+	case PREREQUISITES:
+		size = min_t(u32, common->prerequisites_size, MAX_PREREQUISITES_SIZE);
+
+		for (reqs = 0; reqs < size; reqs++) {
+			if (*elem + reqs >= obj_count) {
+				pr_err("Error elem-objects package is too small\n");
+				return -EINVAL;
+			}
+
+			if (hp_convert_hexstr_to_str(obj[*elem + reqs].string.pointer,
+						     obj[*elem + reqs].string.length,
+						     str_value, &value_len))
+				continue;
+
+			strscpy(common->prerequisites[reqs], *str_value);
+			kfree(*str_value);
+			*str_value = NULL;
+		}
+		if (size)
+			*elem += size - 1;
+		break;
+	case SECURITY_LEVEL:
+		common->security_level = int_value;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 /* map output size to the corresponding WMI method id */
 int hp_encode_outsize_for_pvsz(int outsize)
 {
diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
index ac57d6eab4c3..94619c034187 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
@@ -493,5 +493,8 @@ void hp_friendly_user_name_update(char *path, const char *attr_name,
 				  char *attr_display, int attr_size);
 int hp_wmi_error_and_message(int error_code);
 int hp_get_common_data_from_buffer(u8 **buffer_ptr, u32 *buffer_size, struct common_data *common);
+int hp_get_common_data_from_package(union acpi_object *obj, int obj_count,
+				    int *elem, int *eloc, u32 int_value,
+				    char **str_value, struct common_data *common);
 
 #endif
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index 446dd18d2cee..8f1ecbf127bc 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -132,7 +132,6 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 	u32 size = 0;
 	u32 int_value = 0;
 	int elem = 0;
-	int reqs;
 	int pos_values;
 	int ret;
 	int eloc;
@@ -176,64 +175,18 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 		case VALUE:
 			break;
 		case PATH:
-			strscpy(enum_data->common.path, str_value);
-			break;
 		case IS_READONLY:
-			enum_data->common.is_readonly = int_value;
-			break;
 		case DISPLAY_IN_UI:
-			enum_data->common.display_in_ui = int_value;
-			break;
 		case REQUIRES_PHYSICAL_PRESENCE:
-			enum_data->common.requires_physical_presence = int_value;
-			break;
 		case SEQUENCE:
-			enum_data->common.sequence = int_value;
-			break;
 		case PREREQUISITES_SIZE:
-			if (int_value > MAX_PREREQUISITES_SIZE) {
-				pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
-				int_value = MAX_PREREQUISITES_SIZE;
-			}
-			enum_data->common.prerequisites_size = int_value;
-
-			/*
-			 * This step is needed to keep the expected
-			 * element list pointing to the right obj[elem].type
-			 * when the size is zero. PREREQUISITES
-			 * object is omitted by BIOS when the size is
-			 * zero.
-			 */
-			if (int_value == 0)
-				eloc++;
-			break;
-
 		case PREREQUISITES:
-			size = min_t(u32, enum_data->common.prerequisites_size, MAX_PREREQUISITES_SIZE);
-			for (reqs = 0; reqs < size; reqs++) {
-				if (elem + reqs >= enum_obj_count) {
-					pr_err("Error enum-objects package is too small\n");
-					return -EINVAL;
-				}
-
-				ret = hp_convert_hexstr_to_str(enum_obj[elem + reqs].string.pointer,
-							       enum_obj[elem + reqs].string.length,
-							       &str_value, &value_len);
-
-				if (ret)
-					return -EINVAL;
-
-				strscpy(enum_data->common.prerequisites[reqs], str_value);
-
-				kfree(str_value);
-				str_value = NULL;
-			}
-			if (size)
-				elem += size - 1;
-			break;
-
 		case SECURITY_LEVEL:
-			enum_data->common.security_level = int_value;
+			ret = hp_get_common_data_from_package(enum_obj, enum_obj_count,
+							      &elem, &eloc, int_value, &str_value,
+							      &enum_data->common);
+			if (ret)
+				return ret;
 			break;
 
 		case ENUM_CURRENT_VALUE:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index a27907066448..f2774eff613b 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -145,9 +145,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
 	int ret;
 	u32 int_value = 0;
 	int elem;
-	int reqs;
 	int eloc;
-	int size;
 	struct integer_data *integer_data = &bioscfg_drv.integer_data[instance_id];
 
 	if (!integer_obj)
@@ -192,63 +190,18 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
 			integer_data->current_value = int_value;
 			break;
 		case PATH:
-			strscpy(integer_data->common.path, str_value);
-			break;
 		case IS_READONLY:
-			integer_data->common.is_readonly = int_value;
-			break;
 		case DISPLAY_IN_UI:
-			integer_data->common.display_in_ui = int_value;
-			break;
 		case REQUIRES_PHYSICAL_PRESENCE:
-			integer_data->common.requires_physical_presence = int_value;
-			break;
 		case SEQUENCE:
-			integer_data->common.sequence = int_value;
-			break;
 		case PREREQUISITES_SIZE:
-			if (int_value > MAX_PREREQUISITES_SIZE) {
-				pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
-				int_value = MAX_PREREQUISITES_SIZE;
-			}
-			integer_data->common.prerequisites_size = int_value;
-
-			/*
-			 * This step is needed to keep the expected
-			 * element list pointing to the right obj[elem].type
-			 * when the size is zero. PREREQUISITES
-			 * object is omitted by BIOS when the size is
-			 * zero.
-			 */
-			if (integer_data->common.prerequisites_size == 0)
-				eloc++;
-			break;
 		case PREREQUISITES:
-			size = min_t(u32, integer_data->common.prerequisites_size, MAX_PREREQUISITES_SIZE);
-
-			for (reqs = 0; reqs < size; reqs++) {
-				if (elem + reqs >= integer_obj_count) {
-					pr_err("Error elem-objects package is too small\n");
-					return -EINVAL;
-				}
-
-				ret = hp_convert_hexstr_to_str(integer_obj[elem + reqs].string.pointer,
-							       integer_obj[elem + reqs].string.length,
-							       &str_value, &value_len);
-
-				if (ret)
-					continue;
-
-				strscpy(integer_data->common.prerequisites[reqs], str_value);
-				kfree(str_value);
-				str_value = NULL;
-			}
-			if (size)
-				elem += size - 1;
-			break;
-
 		case SECURITY_LEVEL:
-			integer_data->common.security_level = int_value;
+			ret = hp_get_common_data_from_package(integer_obj, integer_obj_count,
+							      &elem, &eloc, int_value, &str_value,
+							      &integer_data->common);
+			if (ret)
+				return ret;
 			break;
 		case INT_LOWER_BOUND:
 			integer_data->lower_bound = int_value;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 5bf8d40bdf81..95fca0919f1f 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -130,11 +130,9 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 	char *str_value = NULL;
 	int value_len = 0;
 	int ret;
-	u32 size;
 	u32 int_value = 0;
 	int elem;
 	int olist_elem;
-	int reqs;
 	int eloc;
 	char *tmpstr = NULL;
 	char *part_tmp = NULL;
@@ -180,64 +178,18 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 			replace_char_str(ordered_list_data->current_value, COMMA_SEP, SEMICOLON_SEP);
 			break;
 		case PATH:
-			strscpy(ordered_list_data->common.path, str_value);
-			break;
 		case IS_READONLY:
-			ordered_list_data->common.is_readonly = int_value;
-			break;
 		case DISPLAY_IN_UI:
-			ordered_list_data->common.display_in_ui = int_value;
-			break;
 		case REQUIRES_PHYSICAL_PRESENCE:
-			ordered_list_data->common.requires_physical_presence = int_value;
-			break;
 		case SEQUENCE:
-			ordered_list_data->common.sequence = int_value;
-			break;
 		case PREREQUISITES_SIZE:
-			if (int_value > MAX_PREREQUISITES_SIZE) {
-				pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
-				int_value = MAX_PREREQUISITES_SIZE;
-			}
-			ordered_list_data->common.prerequisites_size = int_value;
-
-			/*
-			 * This step is needed to keep the expected
-			 * element list pointing to the right obj[elem].type
-			 * when the size is zero. PREREQUISITES
-			 * object is omitted by BIOS when the size is
-			 * zero.
-			 */
-			if (int_value == 0)
-				eloc++;
-			break;
 		case PREREQUISITES:
-			size = min_t(u32, ordered_list_data->common.prerequisites_size,
-				     MAX_PREREQUISITES_SIZE);
-			for (reqs = 0; reqs < size; reqs++) {
-				if (elem + reqs >= order_obj_count) {
-					pr_err("Error elem-objects package is too small\n");
-					return -EINVAL;
-				}
-
-				ret = hp_convert_hexstr_to_str(order_obj[elem + reqs].string.pointer,
-							       order_obj[elem + reqs].string.length,
-							       &str_value, &value_len);
-
-				if (ret)
-					continue;
-
-				strscpy(ordered_list_data->common.prerequisites[reqs], str_value);
-
-				kfree(str_value);
-				str_value = NULL;
-			}
-			if (size)
-				elem += size - 1;
-			break;
-
 		case SECURITY_LEVEL:
-			ordered_list_data->common.security_level = int_value;
+			ret = hp_get_common_data_from_package(order_obj, order_obj_count,
+							      &elem, &eloc, int_value, &str_value,
+							      &ordered_list_data->common);
+			if (ret)
+				return ret;
 			break;
 
 		case ORD_LIST_SIZE:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index a9e178637416..95fa0aeba920 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -224,7 +224,6 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
 	u32 size;
 	u32 int_value = 0;
 	int elem;
-	int reqs;
 	int eloc;
 	int pos_values;
 	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
@@ -268,64 +267,18 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
 		case VALUE:
 			break;
 		case PATH:
-			strscpy(password_data->common.path, str_value);
-			break;
 		case IS_READONLY:
-			password_data->common.is_readonly = int_value;
-			break;
 		case DISPLAY_IN_UI:
-			password_data->common.display_in_ui = int_value;
-			break;
 		case REQUIRES_PHYSICAL_PRESENCE:
-			password_data->common.requires_physical_presence = int_value;
-			break;
 		case SEQUENCE:
-			password_data->common.sequence = int_value;
-			break;
 		case PREREQUISITES_SIZE:
-			if (int_value > MAX_PREREQUISITES_SIZE) {
-				pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
-				int_value = MAX_PREREQUISITES_SIZE;
-			}
-			password_data->common.prerequisites_size = int_value;
-
-			/* This step is needed to keep the expected
-			 * element list pointing to the right obj[elem].type
-			 * when the size is zero. PREREQUISITES
-			 * object is omitted by BIOS when the size is
-			 * zero.
-			 */
-			if (int_value == 0)
-				eloc++;
-			break;
 		case PREREQUISITES:
-			size = min_t(u32, password_data->common.prerequisites_size,
-				     MAX_PREREQUISITES_SIZE);
-
-			for (reqs = 0; reqs < size; reqs++) {
-				if (elem + reqs >= password_obj_count) {
-					pr_err("Error elem-objects package is too small\n");
-					return -EINVAL;
-				}
-
-				ret = hp_convert_hexstr_to_str(password_obj[elem + reqs].string.pointer,
-							       password_obj[elem + reqs].string.length,
-							       &str_value, &value_len);
-
-				if (ret)
-					break;
-
-				strscpy(password_data->common.prerequisites[reqs], str_value);
-
-				kfree(str_value);
-				str_value = NULL;
-
-			}
-			if (size)
-				elem += size - 1;
-			break;
 		case SECURITY_LEVEL:
-			password_data->common.security_level = int_value;
+			ret = hp_get_common_data_from_package(password_obj, password_obj_count,
+							      &elem, &eloc, int_value, &str_value,
+							      &password_data->common);
+			if (ret)
+				return ret;
 			break;
 		case PSWD_MIN_LENGTH:
 			password_data->min_password_length = int_value;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index f3dfba270f7e..e9bfbef8a3a4 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -135,9 +135,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
 	int ret = 0;
 	u32 int_value = 0;
 	int elem;
-	int reqs;
 	int eloc;
-	int size;
 	struct string_data *string_data = &bioscfg_drv.string_data[instance_id];
 
 	if (!string_obj)
@@ -181,64 +179,18 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
 			strscpy(string_data->current_value, str_value);
 			break;
 		case PATH:
-			strscpy(string_data->common.path, str_value);
-			break;
 		case IS_READONLY:
-			string_data->common.is_readonly = int_value;
-			break;
 		case DISPLAY_IN_UI:
-			string_data->common.display_in_ui = int_value;
-			break;
 		case REQUIRES_PHYSICAL_PRESENCE:
-			string_data->common.requires_physical_presence = int_value;
-			break;
 		case SEQUENCE:
-			string_data->common.sequence = int_value;
-			break;
 		case PREREQUISITES_SIZE:
-			if (int_value > MAX_PREREQUISITES_SIZE) {
-				pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
-				int_value = MAX_PREREQUISITES_SIZE;
-			}
-			string_data->common.prerequisites_size = int_value;
-
-			/*
-			 * This step is needed to keep the expected
-			 * element list pointing to the right obj[elem].type
-			 * when the size is zero. PREREQUISITES
-			 * object is omitted by BIOS when the size is
-			 * zero.
-			 */
-			if (string_data->common.prerequisites_size == 0)
-				eloc++;
-			break;
 		case PREREQUISITES:
-			size = min_t(u32, string_data->common.prerequisites_size,
-				     MAX_PREREQUISITES_SIZE);
-
-			for (reqs = 0; reqs < size; reqs++) {
-				if (elem + reqs >= string_obj_count) {
-					pr_err("Error elem-objects package is too small\n");
-					return -EINVAL;
-				}
-
-				ret = hp_convert_hexstr_to_str(string_obj[elem + reqs].string.pointer,
-							       string_obj[elem + reqs].string.length,
-							       &str_value, &value_len);
-
-				if (ret)
-					continue;
-
-				strscpy(string_data->common.prerequisites[reqs], str_value);
-				kfree(str_value);
-				str_value = NULL;
-			}
-			if (size)
-				elem += size - 1;
-			break;
-
 		case SECURITY_LEVEL:
-			string_data->common.security_level = int_value;
+			ret = hp_get_common_data_from_package(string_obj, string_obj_count,
+							      &elem, &eloc, int_value, &str_value,
+							      &string_data->common);
+			if (ret)
+				return ret;
 			break;
 		case STR_MIN_LENGTH:
 			string_data->min_length = int_value;
-- 
2.55.0