On Thu, 24 Aug 2023 at 22:31, Richard Fitzgerald rf@opensource.cirrus.com wrote:
There is no need to use a test-managed alloc in is_literal(). The function frees the temporary buffer before returning.
This removes the only use of the test and gfp members of struct string_stream outside of the string_stream implementation.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com
This makes sense to me, particularly given how independent string-stream otherwise is from the KUnit resource management bits.
The only possible downside is that the memory won't be cleaned up if strncmp() crashes due to 'text' being somehow invalid. But given this is really only even used with static data (generated by the assert macros), and to fail on the strncmp and not the strlen() would require some horrible race-condition-y madness, I don't think it's ever reasonably possible to hit that case.
So, looks good.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
lib/kunit/assert.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index 05a09652f5a1..dd1d633d0fe2 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -89,8 +89,7 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, EXPORT_SYMBOL_GPL(kunit_ptr_not_err_assert_format);
/* Checks if `text` is a literal representing `value`, e.g. "5" and 5 */ -static bool is_literal(struct kunit *test, const char *text, long long value,
gfp_t gfp)+static bool is_literal(const char *text, long long value) { char *buffer; int len; @@ -100,14 +99,15 @@ static bool is_literal(struct kunit *test, const char *text, long long value, if (strlen(text) != len) return false;
buffer = kunit_kmalloc(test, len+1, gfp);
buffer = kmalloc(len+1, GFP_KERNEL); if (!buffer) return false; snprintf(buffer, len+1, "%lld", value); ret = strncmp(buffer, text, len) == 0;
kunit_kfree(test, buffer);
kfree(buffer);return ret;}
@@ -125,14 +125,12 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, binary_assert->text->left_text, binary_assert->text->operation, binary_assert->text->right_text);
if (!is_literal(stream->test, binary_assert->text->left_text,binary_assert->left_value, stream->gfp))
if (!is_literal(binary_assert->text->left_text, binary_assert->left_value)) string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)\n", binary_assert->text->left_text, binary_assert->left_value, binary_assert->left_value);
if (!is_literal(stream->test, binary_assert->text->right_text,binary_assert->right_value, stream->gfp))
if (!is_literal(binary_assert->text->right_text, binary_assert->right_value)) string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)", binary_assert->text->right_text, binary_assert->right_value,-- 2.30.2