Re: [PATCH v2 bpf-next 4/9] selftests/bpf: free per-cpu values array in bpf_iter selftest

[Hengqi Chen <hengqi.chen@xxxxxxxxx>]

Hi, Andrii

On 2021/11/7 12:03 PM, Andrii Nakryiko wrote:
> Array holding per-cpu values wasn't freed. Fix that.
> 
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
>  tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> index 9454331aaf85..71c724a3f988 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> @@ -770,6 +770,7 @@ static void test_bpf_percpu_hash_map(void)
>  	bpf_link__destroy(link);
>  out:
>  	bpf_iter_bpf_percpu_hash_map__destroy(skel);
> +	free(val);
>  }
>  
>  static void test_bpf_array_map(void)
> 

The val is allocated at the very beginning of this function,
when bpf_iter_bpf_percpu_hash_map__open failed, the val still
leaked.

So we should have:

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 9454331aaf85..ee6727389ef6 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -686,7 +686,7 @@ static void test_bpf_percpu_hash_map(void)
 {
        __u32 expected_key_a = 0, expected_key_b = 0;
        DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
-       struct bpf_iter_bpf_percpu_hash_map *skel;
+       struct bpf_iter_bpf_percpu_hash_map *skel = NULL;
        int err, i, j, len, map_fd, iter_fd;
        union bpf_iter_link_info linfo;
        __u32 expected_val = 0;
@@ -704,7 +704,7 @@ static void test_bpf_percpu_hash_map(void)
        skel = bpf_iter_bpf_percpu_hash_map__open();
        if (CHECK(!skel, "bpf_iter_bpf_percpu_hash_map__open",
                  "skeleton open failed\n"))
-               return;
+               goto out;
 
        skel->rodata->num_cpus = bpf_num_possible_cpus();
 
@@ -770,6 +770,7 @@ static void test_bpf_percpu_hash_map(void)
        bpf_link__destroy(link);
 out:
        bpf_iter_bpf_percpu_hash_map__destroy(skel);
+       free(val);
 }
 
 static void test_bpf_array_map(void)

Right?

Cheers,
--
Hengqi