Re: [PATCH v2 2/4] kunit: Fix the wrong err path and add goto labels in kunit_filter_suites()

On Sun, 3 Sept 2023 at 15:11, 'Jinjie Ruan' via KUnit Development kunit-dev@googlegroups.com wrote:

Take the last kfree(parsed_filters) and add it to be the first. Take the first kfree(copy) and add it to be the last. The Best practice is to return these errors reversely.

And as David suggested, add several labels which target only the things which actually have been allocated so far.

Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Fixes: abbf73816b6f ("kunit: fix possible memory leak in kunit_filter_suites()") Signed-off-by: Jinjie Ruan ruanjinjie@huawei.com Reviewed-by: Rae Moar rmoar@google.com Suggested-by: David Gow davidgow@google.com

---

v2:

• Add err path labels.
• Update the commit message and title.

---

This looks good to me. The kernel test robot does complain about unused labels, so it might make sense to wait to introduce the labels in the later patches (or merge patches 2,3,4 together), but personally I'm okay with it as-is, as these should be merged in one go.

Reviewed-by: David Gow davidgow@google.com

-- David

lib/kunit/executor.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 5181aa2e760b..0eda42b0c9bb 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -166,7 +166,7 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, for (j = 0; j < filter_count; j++) parsed_filters[j] = kunit_next_attr_filter(&filters, err); if (*err)

•                   goto err;
  

•                   goto free_parsed_filters;
    }
  
    for (i = 0; &suite_set->start[i] != suite_set->end; i++) {
  

@@ -178,7 +178,7 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, parsed_glob.test_glob); if (IS_ERR(filtered_suite)) { *err = PTR_ERR(filtered_suite);

•                           goto err;
  

•                           goto free_parsed_filters;
                    }
            }
            if (filter_count > 0 && parsed_filters != NULL) {
  

@@ -195,10 +195,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered_suite = new_filtered_suite;

                            if (*err)

•                                   goto err;
  

•                                   goto free_parsed_filters;
  

•                           if (IS_ERR(filtered_suite)) {
                                    *err = PTR_ERR(filtered_suite);
  

•                                   goto err;
  

•                                   goto free_parsed_filters;
                            }
                            if (!filtered_suite)
                                    break;
  

@@ -213,17 +214,19 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered.start = copy_start; filtered.end = copy;

-err:

•   if (*err)
  

•           kfree(copy);
  

+free_parsed_filters:

•   if (filter_count)
  

•           kfree(parsed_filters);
  
  

+free_parsed_glob: if (filter_glob) { kfree(parsed_glob.suite_glob); kfree(parsed_glob.test_glob); }

•   if (filter_count)
  

•           kfree(parsed_filters);
  

+free_copy:

•   if (*err)
  

•           kfree(copy);
  
    return filtered;
  

}

2.34.1

-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230903071028.1518913-3-ruanjin....