| Message ID | 20220722171534.3576419-5-dlatypov@google.com |
|---|---|
| State | Accepted |
| Commit | 185d57797c5ea82e941befc2489dba0cf162b9c4 |
| Headers | show |
| Series | [v2,1/5] kunit: string-stream: Simplify resource use | expand |
On Sat, Jul 23, 2022 at 1:15 AM Daniel Latypov <dlatypov@google.com> wrote: > > The real kfree() function will silently return when given a NULL. > So a user might reasonably think they can write the following code: > char *buffer = NULL; > if (param->use_buffer) buffer = kunit_kzalloc(test, 10, GFP_KERNEL); > ... > kunit_kfree(test, buffer); > > As-is, kunit_kfree() will mark the test as FAILED when buffer is NULL. > (And in earlier times, it would segfault). > > Let's match the semantics of kfree(). > > Suggested-by: David Gow <davidgow@google.com> > Signed-off-by: Daniel Latypov <dlatypov@google.com> > --- > v1 -> v2: add this patch to the series. > --- Thanks! This looks good to me, and worked with a basic test. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > lib/kunit/test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index c7ca87484968..879c8db36cb5 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -698,6 +698,9 @@ static inline bool kunit_kfree_match(struct kunit *test, > > void kunit_kfree(struct kunit *test, const void *ptr) > { > + if (!ptr) > + return; > + > if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr)) > KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr); > } > -- > 2.37.1.359.gd136c6c3e2-goog >
On Fri, Jul 22, 2022 at 1:15 PM Daniel Latypov <dlatypov@google.com> wrote: > > The real kfree() function will silently return when given a NULL. > So a user might reasonably think they can write the following code: > char *buffer = NULL; > if (param->use_buffer) buffer = kunit_kzalloc(test, 10, GFP_KERNEL); > ... > kunit_kfree(test, buffer); > > As-is, kunit_kfree() will mark the test as FAILED when buffer is NULL. > (And in earlier times, it would segfault). > > Let's match the semantics of kfree(). > > Suggested-by: David Gow <davidgow@google.com> > Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index c7ca87484968..879c8db36cb5 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -698,6 +698,9 @@ static inline bool kunit_kfree_match(struct kunit *test, void kunit_kfree(struct kunit *test, const void *ptr) { + if (!ptr) + return; + if (kunit_destroy_resource(test, kunit_kfree_match, (void *)ptr)) KUNIT_FAIL(test, "kunit_kfree: %px already freed or not allocated by kunit", ptr); }
The real kfree() function will silently return when given a NULL. So a user might reasonably think they can write the following code: char *buffer = NULL; if (param->use_buffer) buffer = kunit_kzalloc(test, 10, GFP_KERNEL); ... kunit_kfree(test, buffer); As-is, kunit_kfree() will mark the test as FAILED when buffer is NULL. (And in earlier times, it would segfault). Let's match the semantics of kfree(). Suggested-by: David Gow <davidgow@google.com> Signed-off-by: Daniel Latypov <dlatypov@google.com> --- v1 -> v2: add this patch to the series. --- lib/kunit/test.c | 3 +++ 1 file changed, 3 insertions(+)