Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <79f81c1c-3ec2-dc0f-56ee-2946987be8b3@redhat.com>
Date:   Wed, 26 Jul 2023 16:28:49 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH 0/5] hp-bioscfg: Address memory leaks and uninitialized
 variable errors
Content-Language: en-US, nl
To:     Jorge Lopez <jorgealtxwork@gmail.com>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
        thomas@t-8ch.de, ilpo.jarvinen@linux.intel.com,
        Dan Carpenter <dan.carpenter@linaro.org>,
        Christophe JAILLET <christophe.jaillet@wanadoo.fr>
References: <20230725220056.25560-1-jorge.lopez2@hp.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20230725220056.25560-1-jorge.lopez2@hp.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Jorge,

On 7/26/23 00:00, Jorge Lopez wrote:
> Submit individual patches to address memory leaks and uninitialized 
> variable errors for each source file listed below.
> 
> - hp_populate_string_elements_from_package()
> drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> 
> - hp_populate_ordered_list_elements_from_package()
> drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> 
> - hp_populate_integer_elements_from_package()
> drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> 
> - hp_populate_enumeration_elements_from_package()
> drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> 
> - hp_populate_password_elements_from_package()
> drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> 
> Changes were tested with a HP EliteBook x360 1030 G3

Thank you for the patches fixing this.

I'm going to wait a bit with applying these to give other people who have looked at these issues a chance to review these patches (added Dan Carpenter and Christophe JAILLET to the To: list).

For future patches please write one patch for each issue you are addressing instead of one patch per file. Since the fixes are the same for all 5 files in this case that would have meant doing 2 patches:

1. To fix the memory leak errors in all 5 files
2. To fix the uninitialized variable in all 5 files.

There is no need to send a new version unless changes are requested by one of the reviewers, I'll just squash all these 5 patches into 1 patch when merging this (and keep the uninitialized variable changes as part of the same patch).

Also these patches should have something like this as part of the commit msg (directly above your Signed-off-by:) :

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/platform-driver-x86/176e14a2-41f8-41b2-854e-77319ec2416d@moroto.mountain/

Regards,

Hans


> 
> 
> Jorge Lopez (5):
>   hp-bioscfg: Fix memory leaks in string_elements_from_package()
>   hp-bioscfg: Fix memory leaks in ordered_list_elements_from_package
>   hp-bioscfg: Fix memory leaks in integer_elements_from_package
>   hp-bioscfg: Fix memory leaks in enumeration_elements_from_package()
>   hp-bioscfg: Fix memory leaks in password_elements_from_package()
> 
>  drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c   |  8 +++++++-
>  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c    |  7 ++++++-
>  .../platform/x86/hp/hp-bioscfg/order-list-attributes.c | 10 ++++++++--
>  .../platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 10 +++++++++-
>  drivers/platform/x86/hp/hp-bioscfg/string-attributes.c |  5 ++++-
>  5 files changed, 34 insertions(+), 6 deletions(-)
>