Commit 6419abb8 authored by Daniel Latypov's avatar Daniel Latypov Committed by Shuah Khan
Browse files

kunit: remove va_format from kunit_assert 

The concern is that having a lot of redundant fields in kunit_assert can
blow up stack usage if the compiler doesn't optimize them away [1].

The comment on this field implies that it was meant to be initialized
when the expect/assert was declared, but this only happens when we run
kunit_do_failed_assertion().

We don't need to access it outside of that function, so move it out of
the struct and make it a local variable there.

This change also takes the chance to reduce the number of macros by
inlining the now simplified KUNIT_INIT_ASSERT_STRUCT() macro.

[1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ



Signed-off-by: default avatarDaniel Latypov <dlatypov@google.com>
Reviewed-by: default avatarDavid Gow <davidgow@google.com>
Reviewed-by: default avatarBrendan Higgins <brendanhiggins@google.com>
Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
parent 95dcbc55

include/kunit/assert.h

+13 −30
Original line number Diff line number Diff line
@@ -42,44 +42,21 @@ struct kunit_loc { 


/**

 * struct kunit_assert - Data for printing a failed assertion or expectation.

 * @message: an optional message to provide additional context.

 * @format: a function which formats the data in this kunit_assert to a string.

 *

 * Represents a failed expectation/assertion. Contains all the data necessary to

 * format a string to a user reporting the failure.

 */

struct kunit_assert {

	struct va_format message;

	void (*format)(const struct kunit_assert *assert,

		       const struct va_format *message,

		       struct string_stream *stream);

};



/**

 * KUNIT_INIT_VA_FMT_NULL - Default initializer for struct va_format.

 *

 * Used inside a struct initialization block to initialize struct va_format to

 * default values where fmt and va are null.

 */

#define KUNIT_INIT_VA_FMT_NULL { .fmt = NULL, .va = NULL }



/**

 * KUNIT_INIT_ASSERT_STRUCT() - Initializer for a &struct kunit_assert.

 * @fmt: The formatting function which builds a string out of this kunit_assert.

 *

 * The base initializer for a &struct kunit_assert.

 */

#define KUNIT_INIT_ASSERT_STRUCT(fmt) {					       \

	.message = KUNIT_INIT_VA_FMT_NULL,				       \

	.format = fmt							       \

}



void kunit_assert_prologue(const struct kunit_loc *loc,

			   enum kunit_assert_type type,

			   struct string_stream *stream);



void kunit_assert_print_msg(const struct kunit_assert *assert,

			    struct string_stream *stream);



/**

 * struct kunit_fail_assert - Represents a plain fail expectation/assertion.

 * @assert: The parent of this type.
@@ -91,6 +68,7 @@ struct kunit_fail_assert { 
};



void kunit_fail_assert_format(const struct kunit_assert *assert,

			      const struct va_format *message,

			      struct string_stream *stream);



/**
@@ -100,7 +78,7 @@ void kunit_fail_assert_format(const struct kunit_assert *assert, 
 * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.

 */

#define KUNIT_INIT_FAIL_ASSERT_STRUCT {					\

	.assert = KUNIT_INIT_ASSERT_STRUCT(kunit_fail_assert_format)	\

	.assert = { .format = kunit_fail_assert_format },		\

}



/**
@@ -120,6 +98,7 @@ struct kunit_unary_assert { 
};



void kunit_unary_assert_format(const struct kunit_assert *assert,

			       const struct va_format *message,

			       struct string_stream *stream);



/**
@@ -131,7 +110,7 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, 
 * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.

 */

#define KUNIT_INIT_UNARY_ASSERT_STRUCT(cond, expect_true) {		       \

	.assert = KUNIT_INIT_ASSERT_STRUCT(kunit_unary_assert_format),	       \

	.assert = { .format = kunit_unary_assert_format },		       \

	.condition = cond,						       \

	.expected_true = expect_true					       \

}
@@ -153,6 +132,7 @@ struct kunit_ptr_not_err_assert { 
};



void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert,

				     const struct va_format *message,

				     struct string_stream *stream);



/**
@@ -165,7 +145,7 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, 
 * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.

 */

#define KUNIT_INIT_PTR_NOT_ERR_STRUCT(txt, val) {			       \

	.assert = KUNIT_INIT_ASSERT_STRUCT(kunit_ptr_not_err_assert_format),   \

	.assert = { .format = kunit_ptr_not_err_assert_format },	       \

	.text = txt,							       \

	.value = val							       \

}
@@ -194,6 +174,7 @@ struct kunit_binary_assert { 
};



void kunit_binary_assert_format(const struct kunit_assert *assert,

				const struct va_format *message,

				struct string_stream *stream);



/**
@@ -213,7 +194,7 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, 
					left_val,			       \

					right_str,			       \

					right_val) {			       \

	.assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_assert_format),	       \

	.assert = { .format = kunit_binary_assert_format },		       \

	.operation = op_str,						       \

	.left_text = left_str,						       \

	.left_value = left_val,						       \
@@ -245,6 +226,7 @@ struct kunit_binary_ptr_assert { 
};



void kunit_binary_ptr_assert_format(const struct kunit_assert *assert,

				    const struct va_format *message,

				    struct string_stream *stream);



/**
@@ -265,7 +247,7 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, 
					    left_val,			       \

					    right_str,			       \

					    right_val) {		       \

	.assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_ptr_assert_format),    \

	.assert = { .format = kunit_binary_ptr_assert_format },		       \

	.operation = op_str,						       \

	.left_text = left_str,						       \

	.left_value = left_val,						       \
@@ -297,6 +279,7 @@ struct kunit_binary_str_assert { 
};



void kunit_binary_str_assert_format(const struct kunit_assert *assert,

				    const struct va_format *message,

				    struct string_stream *stream);



/**
@@ -316,7 +299,7 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, 
					    left_val,			       \

					    right_str,			       \

					    right_val) {		       \

	.assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_str_assert_format),    \

	.assert = { .format = kunit_binary_str_assert_format },		       \

	.operation = op_str,						       \

	.left_text = left_str,						       \

	.left_value = left_val,						       \

lib/kunit/assert.c

+16 −11
Original line number Diff line number Diff line
@@ -30,22 +30,23 @@ void kunit_assert_prologue(const struct kunit_loc *loc, 
}

EXPORT_SYMBOL_GPL(kunit_assert_prologue);



void kunit_assert_print_msg(const struct kunit_assert *assert,

static void kunit_assert_print_msg(const struct va_format *message,

				   struct string_stream *stream)

{

	if (assert->message.fmt)

		string_stream_add(stream, "\n%pV", &assert->message);

	if (message->fmt)

		string_stream_add(stream, "\n%pV", message);

}

EXPORT_SYMBOL_GPL(kunit_assert_print_msg);



void kunit_fail_assert_format(const struct kunit_assert *assert,

			      const struct va_format *message,

			      struct string_stream *stream)

{

	string_stream_add(stream, "%pV", &assert->message);

	string_stream_add(stream, "%pV", message);

}

EXPORT_SYMBOL_GPL(kunit_fail_assert_format);



void kunit_unary_assert_format(const struct kunit_assert *assert,

			       const struct va_format *message,

			       struct string_stream *stream)

{

	struct kunit_unary_assert *unary_assert;
@@ -60,11 +61,12 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, 
		string_stream_add(stream,

				  KUNIT_SUBTEST_INDENT "Expected %s to be false, but is true\n",

				  unary_assert->condition);

	kunit_assert_print_msg(assert, stream);

	kunit_assert_print_msg(message, stream);

}

EXPORT_SYMBOL_GPL(kunit_unary_assert_format);



void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert,

				     const struct va_format *message,

				     struct string_stream *stream)

{

	struct kunit_ptr_not_err_assert *ptr_assert;
@@ -82,7 +84,7 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, 
				  ptr_assert->text,

				  PTR_ERR(ptr_assert->value));

	}

	kunit_assert_print_msg(assert, stream);

	kunit_assert_print_msg(message, stream);

}

EXPORT_SYMBOL_GPL(kunit_ptr_not_err_assert_format);
@@ -110,6 +112,7 @@ static bool is_literal(struct kunit *test, const char *text, long long value, 
}



void kunit_binary_assert_format(const struct kunit_assert *assert,

				const struct va_format *message,

				struct string_stream *stream)

{

	struct kunit_binary_assert *binary_assert;
@@ -132,11 +135,12 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, 
		string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld",

				  binary_assert->right_text,

				  binary_assert->right_value);

	kunit_assert_print_msg(assert, stream);

	kunit_assert_print_msg(message, stream);

}

EXPORT_SYMBOL_GPL(kunit_binary_assert_format);



void kunit_binary_ptr_assert_format(const struct kunit_assert *assert,

				    const struct va_format *message,

				    struct string_stream *stream)

{

	struct kunit_binary_ptr_assert *binary_assert;
@@ -155,7 +159,7 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, 
	string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px",

			  binary_assert->right_text,

			  binary_assert->right_value);

	kunit_assert_print_msg(assert, stream);

	kunit_assert_print_msg(message, stream);

}

EXPORT_SYMBOL_GPL(kunit_binary_ptr_assert_format);
@@ -176,6 +180,7 @@ static bool is_str_literal(const char *text, const char *value) 
}



void kunit_binary_str_assert_format(const struct kunit_assert *assert,

				    const struct va_format *message,

				    struct string_stream *stream)

{

	struct kunit_binary_str_assert *binary_assert;
@@ -196,6 +201,6 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, 
		string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"",

				  binary_assert->right_text,

				  binary_assert->right_value);

	kunit_assert_print_msg(assert, stream);

	kunit_assert_print_msg(message, stream);

}

EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format);

lib/kunit/test.c

+7 −5
Original line number Diff line number Diff line
@@ -241,7 +241,8 @@ static void kunit_print_string_stream(struct kunit *test, 
}



static void kunit_fail(struct kunit *test, const struct kunit_loc *loc,

		       enum kunit_assert_type type, struct kunit_assert *assert)

		       enum kunit_assert_type type, struct kunit_assert *assert,

		       const struct va_format *message)

{

	struct string_stream *stream;
@@ -257,7 +258,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc, 
	}



	kunit_assert_prologue(loc, type, stream);

	assert->format(assert, stream);

	assert->format(assert, message, stream);



	kunit_print_string_stream(test, stream);
@@ -284,12 +285,13 @@ void kunit_do_failed_assertion(struct kunit *test, 
			       const char *fmt, ...)

{

	va_list args;

	struct va_format message;

	va_start(args, fmt);



	assert->message.fmt = fmt;

	assert->message.va = &args;

	message.fmt = fmt;

	message.va = &args;



	kunit_fail(test, loc, type, assert);

	kunit_fail(test, loc, type, assert, &message);



	va_end(args);