[2/3] test/hash: free allocated memory

Message ID 20190627032420.4730-2-honnappa.nagarahalli@arm.com
State New
Headers show
Series
  • [1/3] test/hash: reset global variable to discard old data
Related show

Commit Message

Honnappa Nagarahalli June 27, 2019, 3:24 a.m.
Free allocated memory.

Fixes: 3f9aab961ed3 ("test/hash: check lock-free extendable bucket")
Cc: stable@dpdk.org

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>

---
 app/test/test_hash_readwrite_lf.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.17.1

Comments

David Marchand June 27, 2019, 8:07 a.m. | #1
On Thu, Jun 27, 2019 at 5:25 AM Honnappa Nagarahalli <
honnappa.nagarahalli@arm.com> wrote:

> Free allocated memory.

>

> Fixes: 3f9aab961ed3 ("test/hash: check lock-free extendable bucket")

> Cc: stable@dpdk.org

>

> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>

> ---

>  app/test/test_hash_readwrite_lf.c | 2 ++

>  1 file changed, 2 insertions(+)

>

> diff --git a/app/test/test_hash_readwrite_lf.c

> b/app/test/test_hash_readwrite_lf.c

> index e9aca6ff4..e92d1065b 100644

> --- a/app/test/test_hash_readwrite_lf.c

> +++ b/app/test/test_hash_readwrite_lf.c

> @@ -1431,6 +1431,8 @@ test_hash_readwrite_lf_main(void)

>         rte_free(tbl_rwc_test_param.keys_ks);

>         rte_free(tbl_rwc_test_param.keys_absent);

>         rte_free(tbl_rwc_test_param.keys_shift_path);

> +       rte_free(tbl_rwc_test_param.keys_ext_bkt);

> +       rte_free(tbl_rwc_test_param.keys_ks_extbkt);

>         rte_free(scanned_bkts);

>         return 0;

>  }

> --

> 2.17.1

>



I inspected this test a little more, I can see other leaks.
- generate_keys(), on error, we are leaking tbl_rwc_test_param.h.
- test_rwc_reader(), we are leaking pos.
- test_hash_readwrite_lf_main(), we are leaking
tbl_rwc_test_param.keys_non_shift_path

Looking at your patch 1, I would prefer you reset tbl_rwc_test_param field
per field to NULL once the rte_free() is called.
It would then be easier for you to track the remaining leaks (and patch 1
disappears).


-- 
David Marchand
Honnappa Nagarahalli June 28, 2019, 4:59 a.m. | #2
On Thu, Jun 27, 2019 at 5:25 AM Honnappa Nagarahalli <honnappa.nagarahalli@arm.com<mailto:honnappa.nagarahalli@arm.com>> wrote:
Free allocated memory.

Fixes: 3f9aab961ed3 ("test/hash: check lock-free extendable bucket")
Cc: stable@dpdk.org<mailto:stable@dpdk.org>

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com<mailto:honnappa.nagarahalli@arm.com>>

Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:dharmik.thakkar@arm.com>>

---
 app/test/test_hash_readwrite_lf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index e9aca6ff4..e92d1065b 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -1431,6 +1431,8 @@ test_hash_readwrite_lf_main(void)
        rte_free(tbl_rwc_test_param.keys_ks);
        rte_free(tbl_rwc_test_param.keys_absent);
        rte_free(tbl_rwc_test_param.keys_shift_path);
+       rte_free(tbl_rwc_test_param.keys_ext_bkt);
+       rte_free(tbl_rwc_test_param.keys_ks_extbkt);
        rte_free(scanned_bkts);
        return 0;
 }
--
2.17.1


I inspected this test a little more, I can see other leaks.
- generate_keys(), on error, we are leaking tbl_rwc_test_param.h.
- test_rwc_reader(), we are leaking pos.
- test_hash_readwrite_lf_main(), we are leaking tbl_rwc_test_param.keys_non_shift_path
[Honnappa] Thanks for the review. I fixed these and few more in V2.

Looking at your patch 1, I would prefer you reset tbl_rwc_test_param field per field to NULL once the rte_free() is called.
It would then be easier for you to track the remaining leaks (and patch 1 disappears).
[Honnappa] This can be considered as initialization code. I have left this as it is.


--
David Marchand
David Marchand June 28, 2019, 8:36 a.m. | #3
On Fri, Jun 28, 2019 at 6:59 AM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:

>

> On Thu, Jun 27, 2019 at 5:25 AM Honnappa Nagarahalli <

> honnappa.nagarahalli@arm.com<mailto:honnappa.nagarahalli@arm.com>> wrote:

> Free allocated memory.

>

> Fixes: 3f9aab961ed3 ("test/hash: check lock-free extendable bucket")

> Cc: stable@dpdk.org<mailto:stable@dpdk.org>

>

> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com<mailto:

> honnappa.nagarahalli@arm.com>>

> Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com<mailto:

> dharmik.thakkar@arm.com>>

> ---

>  app/test/test_hash_readwrite_lf.c | 2 ++

>  1 file changed, 2 insertions(+)

>

> diff --git a/app/test/test_hash_readwrite_lf.c

> b/app/test/test_hash_readwrite_lf.c

> index e9aca6ff4..e92d1065b 100644

> --- a/app/test/test_hash_readwrite_lf.c

> +++ b/app/test/test_hash_readwrite_lf.c

> @@ -1431,6 +1431,8 @@ test_hash_readwrite_lf_main(void)

>         rte_free(tbl_rwc_test_param.keys_ks);

>         rte_free(tbl_rwc_test_param.keys_absent);

>         rte_free(tbl_rwc_test_param.keys_shift_path);

> +       rte_free(tbl_rwc_test_param.keys_ext_bkt);

> +       rte_free(tbl_rwc_test_param.keys_ks_extbkt);

>         rte_free(scanned_bkts);

>         return 0;

>  }

> --

> 2.17.1

>

>

> I inspected this test a little more, I can see other leaks.

> - generate_keys(), on error, we are leaking tbl_rwc_test_param.h.

> - test_rwc_reader(), we are leaking pos.

> - test_hash_readwrite_lf_main(), we are leaking

> tbl_rwc_test_param.keys_non_shift_path

> [Honnappa] Thanks for the review. I fixed these and few more in V2.

>


Still missed pos in some err path.
Comment incoming.


> Looking at your patch 1, I would prefer you reset tbl_rwc_test_param field

> per field to NULL once the rte_free() is called.

> It would then be easier for you to track the remaining leaks (and patch 1

> disappears).

> [Honnappa] This can be considered as initialization code. I have left this

> as it is.

>


Ok, I won't insist.

-- 
David Marchand

Patch

diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index e9aca6ff4..e92d1065b 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -1431,6 +1431,8 @@  test_hash_readwrite_lf_main(void)
 	rte_free(tbl_rwc_test_param.keys_ks);
 	rte_free(tbl_rwc_test_param.keys_absent);
 	rte_free(tbl_rwc_test_param.keys_shift_path);
+	rte_free(tbl_rwc_test_param.keys_ext_bkt);
+	rte_free(tbl_rwc_test_param.keys_ks_extbkt);
 	rte_free(scanned_bkts);
 	return 0;
 }