Submit individual patches to address memory leaks and uninitialized 
variable errors. 
Addressed several review comments making the source code more readable.
Removed duplicate use of variable in inner loop.

Changes were tested with a HP EliteBook x360 1030 G3

Jorge Lopez (8):
  hp-bioscfg: Fix memory leaks in attribute packages
  hp-bioscfg: Fix uninitialized variable errors
  hp-bioscfg: Replace the word HACK from source code
  hp-bioscfg: Change how prerequisites size is evaluated
  hp-bioscfg: Change how order list size is evaluated
  hp-bioscfg: Change how enum possible values size is evaluated
  hp-bioscfg: Change how password encoding size is evaluated
  hp-bioscfg: Remove duplicate use of variable in inner loop

 .../x86/hp/hp-bioscfg/enum-attributes.c       | 24 ++++++++----
 .../x86/hp/hp-bioscfg/int-attributes.c        | 15 +++++--
 .../x86/hp/hp-bioscfg/order-list-attributes.c | 39 ++++++++++++-------
 .../x86/hp/hp-bioscfg/passwdobj-attributes.c  | 27 +++++++++----
 .../x86/hp/hp-bioscfg/string-attributes.c     | 13 +++++--
 5 files changed, 82 insertions(+), 36 deletions(-)

--
2.34.1

These are fine.  We still need to do something like this.  Also we could
just get rid of value_len completely.  Nothing uses it.

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 cffc1c9ba3e77..6ba0e49e787ec 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -264,7 +264,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 			 * Ordered list data is stored in hex and comma separated format
 			 * Convert the data and split it to show each element
 			 */
-			ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);
+			ret = hp_convert_hexstr_to_str(str_value, size, &tmpstr, &tmp_len);
 			if (ret)
 				goto exit_list;

I will submit a new patch replacing 'value_len' for 'size' in line 267
as indicated.
'value_len' is utilized earlier in the code so we cannot remove it
completely from the function.


On Tue, Aug 1, 2023 at 8:35 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> These are fine.  We still need to do something like this.  Also we could
> just get rid of value_len completely.  Nothing uses it.
>
> 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 cffc1c9ba3e77..6ba0e49e787ec 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> @@ -264,7 +264,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>                          * Ordered list data is stored in hex and comma separated format
>                          * Convert the data and split it to show each element
>                          */
> -                       ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);
> +                       ret = hp_convert_hexstr_to_str(str_value, size, &tmpstr, &tmp_len);
>                         if (ret)
>                                 goto exit_list;
>

On Tue, Aug 01, 2023 at 09:52:05AM -0500, Jorge Lopez wrote:
> I will submit a new patch replacing 'value_len' for 'size' in line 267
> as indicated.
> 'value_len' is utilized earlier in the code so we cannot remove it
> completely from the function.
> 

After replacing size then it looks like this.

$ grep value_len drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
        int value_len = 0;
                                                               &str_value, &value_len);
                                                               &str_value, &value_len);

It's a write only variable.

regards,
dan carpenter

Ok. Thanks for the clarification.  I will remove 'value_len' and
replace all its references with 'size'.

On Tue, Aug 1, 2023 at 10:04 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Tue, Aug 01, 2023 at 09:52:05AM -0500, Jorge Lopez wrote:
> > I will submit a new patch replacing 'value_len' for 'size' in line 267
> > as indicated.
> > 'value_len' is utilized earlier in the code so we cannot remove it
> > completely from the function.
> >
>
> After replacing size then it looks like this.
>
> $ grep value_len drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
>         int value_len = 0;
>                                                                &str_value, &value_len);
>                                                                &str_value, &value_len);
>
> It's a write only variable.
>
> regards,
> dan carpenter
>

On Tue, Aug 01, 2023 at 10:10:05AM -0500, Jorge Lopez wrote:
> Ok. Thanks for the clarification.  I will remove 'value_len' and
> replace all its references with 'size'.

Ugh...  No, that's worse than the original.  Just leave value_len as is
in that case.  :P

regards,
dan carpenter

Hi,

On 7/31/23 22:31, Jorge Lopez wrote:
> Submit individual patches to address memory leaks and uninitialized 
> variable errors. 
> Addressed several review comments making the source code more readable.
> Removed duplicate use of variable in inner loop.
> 
> Changes were tested with a HP EliteBook x360 1030 G3
> 
> Jorge Lopez (8):
>   hp-bioscfg: Fix memory leaks in attribute packages
>   hp-bioscfg: Fix uninitialized variable errors
>   hp-bioscfg: Replace the word HACK from source code
>   hp-bioscfg: Change how prerequisites size is evaluated
>   hp-bioscfg: Change how order list size is evaluated
>   hp-bioscfg: Change how enum possible values size is evaluated
>   hp-bioscfg: Change how password encoding size is evaluated
>   hp-bioscfg: Remove duplicate use of variable in inner loop
> 
>  .../x86/hp/hp-bioscfg/enum-attributes.c       | 24 ++++++++----
>  .../x86/hp/hp-bioscfg/int-attributes.c        | 15 +++++--
>  .../x86/hp/hp-bioscfg/order-list-attributes.c | 39 ++++++++++++-------
>  .../x86/hp/hp-bioscfg/passwdobj-attributes.c  | 27 +++++++++----
>  .../x86/hp/hp-bioscfg/string-attributes.c     | 13 +++++--
>  5 files changed, 82 insertions(+), 36 deletions(-)

Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans