Re: [PATCH 11/13] platform/x86: hp-bioscfg: advance elem past consumed array elements in string-attributes

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 3 Aug 2026, Muhammad Bilal wrote:

> Same defect as the companion fix to enum-attributes.c in this series:
> the PREREQUISITES case consumes "size" consecutive ACPI package
> elements via elem + reqs, but the outer loop only advances "elem" by
> one per iteration, causing the next iteration to misread a leftover
> prerequisite entry as the next property and abort the parse with
> -EIO on the resulting type mismatch.
> 
> Fix by advancing "elem" by (size - 1) after the loop.
> 
> Fixes: e6c7b3e15559 ("platform/x86: hp-bioscfg: string-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-bioscfg/string-attributes.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> index fe5a9a3a4ef1..5abec8995911 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> @@ -233,6 +233,8 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
>  				kfree(str_value);
>  				str_value = NULL;
>  			}
> +			if (size)
> +				elem += size - 1;
>  			break;
>  
>  		case SECURITY_LEVEL:
> 

Hi,

First of all, thanks for working with this driver.

There's no need to do the same fix in n patches. If the problem is the 
same in multiple files, they can be in the same patch. You just need to 
generalize your shortlog prefix a bit when combining.

I'll probably take this series once that is fixed (and after I do a 
deeper review on them changes) ...

... BUT,

This entire driver was merged half-baked, with outstanding comments from 
me unaddressed. One of the most important unaddressed comment related to 
the massive use of copy-pasted code which leads to having the same bug in 
multiple places.

Once this series is done, could you try to look into consolidating similar 
parts in these attribute type files so that there would be a common 
function that is called by each of these attribute type files (and if a 
common function is not possible due to types, perhaps macro could be tried 
instead)? I'd be very welcoming for such a cleanup to finally start 
cleaning up the mess this driver is.

If you extract e.g. hp_populate_integer_elements_from_package() and 
hp_populate_string_elements_from_package() and diff those two functions, 
you'll see what I mean. A struct is even called "common" in the struct
string/integer_data so I expect it could be parsed by a common function.

-- 
 i.