On Thu, 31 Aug 2023 at 15:17, 'Jinjie Ruan' via KUnit Development
<kunit-dev@xxxxxxxxxxxxxxxx> 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.
>
> 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@xxxxxxxxxx>
> ---
Agreed, these should be freed in reverse order.
Would it make sense to initialise 'copy' to NULL, and free it if (copy
!= NULL), rather than if (*err)? As mentioned in the previous patch, I
think that'd be more correct.
We could also have several labels which target only the things which
actually have been allocated so far.
Thoughts?
-- David
> lib/kunit/executor.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 7654c09c1ab1..da9444d22711 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -229,16 +229,16 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
> filtered.end = copy;
>
> err:
> - if (*err)
> - kfree(copy);
> + if (filter_count)
> + kfree(parsed_filters);
>
> if (filter_glob) {
> kfree(parsed_glob.suite_glob);
> kfree(parsed_glob.test_glob);
> }
I think this might also be potentially broken. If
parsed_glob.{suite,test}_glob are not successfully allocated,
filter_glob will still be set, and we'll kfree() something invalid.
Should we also init parsed_glob.* to NULL, and free them if non-NULL,
rather than relying on the presence of filter_glob?
>
> - if (filter_count)
> - kfree(parsed_filters);
> + 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@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230831071655.2907683-5-ruanjinjie%40huawei.com.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
