Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201228193351.Bluez.v3.1.Ie32770d0eed2e7739ce9d17d920766fb6aee8583@changeid>
 <20201228193351.Bluez.v3.4.I43884adadc00a5095dd03d2261a71dc2ba80d986@changeid>
In-Reply-To: <20201228193351.Bluez.v3.4.I43884adadc00a5095dd03d2261a71dc2ba80d986@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Sun, 3 Jan 2021 23:00:10 -0800
Message-ID: <CABBYNZJdy2iL8gezxd6gOwkSUtxTr3VQvrVehU7ec1GV3twxXw@mail.gmail.com>
Subject: Re: [Bluez PATCH v3 4/4] shared/mgmt: Fix memory leak in mgmt_tlv_list
To:     Howard Chung <howardchung@google.com>
Cc:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        Miao-chen Chou <mcchou@chromium.org>,
        Manish Mandlik <mmandlik@chromium.org>,
        Archie Pusaka <apusaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Howard,

On Mon, Dec 28, 2020 at 3:34 AM Howard Chung <howardchung@google.com> wrote:
>
> This patch freed the mgmt_tlv properly in mgmt_tlv_list_free.
>
> Reviewed-by: apusaka@chromium.org
> Reviewed-by: mcchou@chromium.org
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Fix incompatible pointer type error of mgmt_tlv_free
>
>  src/shared/mgmt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
> index dc8107846668..0d0c957709d7 100644
> --- a/src/shared/mgmt.c
> +++ b/src/shared/mgmt.c
> @@ -588,14 +588,15 @@ static struct mgmt_tlv *mgmt_tlv_new(uint16_t type, uint8_t length,
>         return entry;
>  }
>
> -static void mgmt_tlv_free(struct mgmt_tlv *entry)
> +static void mgmt_tlv_free(void *data)
>  {
> +       struct mgmt_tlv *entry = data;
>         free(entry);
>  }
>
>  void mgmt_tlv_list_free(struct mgmt_tlv_list *tlv_list)
>  {
> -       queue_destroy(tlv_list->tlv_queue, NULL);
> +       queue_destroy(tlv_list->tlv_queue, mgmt_tlv_free);

It might be better to just pass free directly instead of mgmt_tlv_free
since all it does currently is call free anyway.

>         free(tlv_list);
>  }
>
> --
> 2.29.2.729.g45daf8777d-goog
>


-- 
Luiz Augusto von Dentz