Re: [PATCH bpf-next] selftests/bpf: fix trampoline_count clean up logic

On Feb 20, 2020, at 8:20 PM, Andrii Nakryiko [off-list ref] wrote:

On 2/20/20 6:06 PM, Song Liu wrote:

quoted

On Thu, Feb 20, 2020 at 3:07 PM Andrii Nakryiko [off-list ref] wrote:

quoted

Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
clean up is performed correctly.

Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <redacted>
---
 .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
index 1f6ccdaed1ac..781c8d11604b 100644
--- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
+++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c

@@ -55,31 +55,40 @@ void test_trampoline_count(void)
        /* attach 'allowed' 40 trampoline programs */
        for (i = 0; i < MAX_TRAMP_PROGS; i++) {
                obj = bpf_object__open_file(object, NULL);
-               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
+               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
+                       obj = NULL;

I think we don't need obj and link in cleanup? Did I miss anything?

We do set obj below (line 87) after this loop, so need to clean it up as well. As for link, yeah, technically link doesn't have to be set to NULL, but I kind of did it for completeness without thinking too much.

I meant "obj = NULL;" before "goto cleanup;", as we don't use obj in the 
cleanup path. 

Anyway, this is not a real issue. 

Thanks,
Song