This patch chain changes the logging implementation to use string_stream
so that the log will grow dynamically.
The first 8 patches add test code for string_stream, and make some
changes to string_stream needed to be able to use it for the log.
The final patch adds a performance report of string_stream.
CHANGES SINCE V3:
Completely rewritten to use string_stream instead of implementing a
separate extending-buffer implementation for logging.
I have used the performance test from the final patch on my original
fixed-size-fragment implementation from V3 to get a comparison of the
two implementations (run on i3-8145UE CPU @ 2.20GHz):
string_stream V3 fixed-size-buffer
Time elapsed: 7748 us 3251 us
Total string length: 573890 573890
Bytes requested: 823994 728336
Actual bytes allocated: 1061440 728352
I don't think the difference is enough to be worth complicating the
string_stream implementation with my fixed-fragment implementation from
V3 of this patch chain.
Richard Fitzgerald (10):
kunit: string-stream: Improve testing of string_stream
kunit: string-stream: Don't create a fragment for empty strings
kunit: string-stream: Add cases for adding empty strings to a
string_stream
kunit: string-stream: Add option to make all lines end with newline
kunit: string-stream: Add cases for string_stream newline appending
kunit: string-stream: Pass struct kunit to string_stream_get_string()
kunit: string-stream: Decouple string_stream from kunit
kunit: string-stream: Add test for freeing resource-managed
string_stream
kunit: Use string_stream for test log
kunit: string-stream: Test performance of string_stream
include/kunit/test.h | 14 +-
lib/kunit/Makefile | 5 +-
lib/kunit/debugfs.c | 36 ++-
lib/kunit/kunit-test.c | 52 +---
lib/kunit/log-test.c | 72 ++++++
lib/kunit/string-stream-test.c | 447 +++++++++++++++++++++++++++++++--
lib/kunit/string-stream.c | 129 +++++++---
lib/kunit/string-stream.h | 22 +-
lib/kunit/test.c | 48 +---
9 files changed, 656 insertions(+), 169 deletions(-)
create mode 100644 lib/kunit/log-test.c
--
2.30.2
If the result of the formatted string is an empty string just return
instead of creating an empty fragment.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
index cc32743c1171..ed24d86af9f5 100644
--- a/lib/kunit/string-stream.c
+++ b/lib/kunit/string-stream.c
@@ -50,11 +50,17 @@ int string_stream_vadd(struct string_stream *stream,
/* Make a copy because `vsnprintf` could change it */
va_copy(args_for_counting, args);
- /* Need space for null byte. */
- len = vsnprintf(NULL, 0, fmt, args_for_counting) + 1;
+ /* Evaluate length of formatted string */
+ len = vsnprintf(NULL, 0, fmt, args_for_counting);
va_end(args_for_counting);
+ if (len == 0)
+ return 0;
+
+ /* Need space for null byte. */
+ len++;
+
frag_container = alloc_string_stream_fragment(stream->test,
len,
stream->gfp);
--
2.30.2
Add a test of the speed and memory use of string_stream.
string_stream_performance_test() doesn't actually "test" anything (it
cannot fail unless the system has run out of allocatable memory) but it
measures the speed and memory consumption of the string_stream and reports
the result.
This allows changes in the string_stream implementation to be compared.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream-test.c | 54 ++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
index 05bfade2bd8a..b55cc14f43fb 100644
--- a/lib/kunit/string-stream-test.c
+++ b/lib/kunit/string-stream-test.c
@@ -8,7 +8,9 @@
#include <kunit/static_stub.h>
#include <kunit/test.h>
+#include <linux/ktime.h>
#include <linux/slab.h>
+#include <linux/timekeeping.h>
#include "string-stream.h"
@@ -370,6 +372,57 @@ static void string_stream_auto_newline_test(struct kunit *test)
"One\nTwo\nThree\nFour\nFive\nSix\nSeven\n\nEight\n");
}
+/*
+ * This doesn't actually "test" anything. It reports time taken
+ * and memory used for logging a large number of lines.
+ */
+static void string_stream_performance_test(struct kunit *test)
+{
+ struct string_stream_fragment *frag_container;
+ struct string_stream *stream;
+ char test_line[101];
+ ktime_t start_time, end_time;
+ size_t len, bytes_requested, actual_bytes_used, total_string_length;
+ int offset, i;
+
+ stream = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
+
+ memset(test_line, 'x', sizeof(test_line) - 1);
+ test_line[sizeof(test_line) - 1] = '\0';
+
+ start_time = ktime_get();
+ for (i = 0; i < 10000; i++) {
+ offset = i % (sizeof(test_line) - 1);
+ string_stream_add(stream, "%s: %d\n", &test_line[offset], i);
+ }
+ end_time = ktime_get();
+
+ /*
+ * Calculate memory used. This doesn't include invisible
+ * overhead due to kernel allocator fragment size rounding.
+ */
+ bytes_requested = sizeof(*stream);
+ actual_bytes_used = ksize(stream);
+ total_string_length = 0;
+
+ list_for_each_entry(frag_container, &stream->fragments, node) {
+ bytes_requested += sizeof(*frag_container);
+ actual_bytes_used += ksize(frag_container);
+
+ len = strlen(frag_container->fragment);
+ total_string_length += len;
+ bytes_requested += len + 1; /* +1 for '\0' */
+ actual_bytes_used += ksize(frag_container->fragment);
+ }
+
+ kunit_info(test, "Time elapsed: %lld us\n",
+ ktime_us_delta(end_time, start_time));
+ kunit_info(test, "Total string length: %zu\n", total_string_length);
+ kunit_info(test, "Bytes requested: %zu\n", bytes_requested);
+ kunit_info(test, "Actual bytes allocated: %zu\n", actual_bytes_used);
+}
+
static int string_stream_test_init(struct kunit *test)
{
struct string_stream_test_priv *priv;
@@ -400,6 +453,7 @@ static struct kunit_case string_stream_test_cases[] = {
KUNIT_CASE(string_stream_append_empty_string_test),
KUNIT_CASE(string_stream_no_auto_newline_test),
KUNIT_CASE(string_stream_auto_newline_test),
+ KUNIT_CASE(string_stream_performance_test),
{}
};
--
2.30.2
Add an optional feature to string_stream that will append a newline to
any added string that does not already end with a newline. The purpose
of this is so that string_stream can be used to collect log lines.
This is enabled/disabled by calling string_stream_set_append_newlines().
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream.c | 28 +++++++++++++++++++++-------
lib/kunit/string-stream.h | 7 +++++++
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
index ed24d86af9f5..1dcf6513b692 100644
--- a/lib/kunit/string-stream.c
+++ b/lib/kunit/string-stream.c
@@ -44,32 +44,46 @@ int string_stream_vadd(struct string_stream *stream,
va_list args)
{
struct string_stream_fragment *frag_container;
- int len;
+ int buf_len, result_len;
va_list args_for_counting;
/* Make a copy because `vsnprintf` could change it */
va_copy(args_for_counting, args);
/* Evaluate length of formatted string */
- len = vsnprintf(NULL, 0, fmt, args_for_counting);
+ buf_len = vsnprintf(NULL, 0, fmt, args_for_counting);
va_end(args_for_counting);
- if (len == 0)
+ if (buf_len == 0)
return 0;
+ /* Reserve one extra for possible appended newline. */
+ if (stream->append_newlines)
+ buf_len++;
+
/* Need space for null byte. */
- len++;
+ buf_len++;
frag_container = alloc_string_stream_fragment(stream->test,
- len,
+ buf_len,
stream->gfp);
if (IS_ERR(frag_container))
return PTR_ERR(frag_container);
- len = vsnprintf(frag_container->fragment, len, fmt, args);
+ if (stream->append_newlines) {
+ /* Don't include reserved newline byte in writeable length. */
+ result_len = vsnprintf(frag_container->fragment, buf_len - 1, fmt, args);
+
+ /* Append newline if necessary. */
+ if (frag_container->fragment[result_len - 1] != '\n')
+ result_len = strlcat(frag_container->fragment, "\n", buf_len);
+ } else {
+ result_len = vsnprintf(frag_container->fragment, buf_len, fmt, args);
+ }
+
spin_lock(&stream->lock);
- stream->length += len;
+ stream->length += result_len;
list_add_tail(&frag_container->node, &stream->fragments);
spin_unlock(&stream->lock);
diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h
index b669f9a75a94..048930bf97f0 100644
--- a/lib/kunit/string-stream.h
+++ b/lib/kunit/string-stream.h
@@ -25,6 +25,7 @@ struct string_stream {
spinlock_t lock;
struct kunit *test;
gfp_t gfp;
+ bool append_newlines;
};
struct kunit;
@@ -47,4 +48,10 @@ bool string_stream_is_empty(struct string_stream *stream);
void string_stream_destroy(struct string_stream *stream);
+static inline void string_stream_set_append_newlines(struct string_stream *stream,
+ bool append_newlines)
+{
+ stream->append_newlines = append_newlines;
+}
+
#endif /* _KUNIT_STRING_STREAM_H */
--
2.30.2
Pass a struct kunit* and gfp_t to string_stream_get_string(). Allocate
the returned buffer using these instead of using the stream->test and
stream->gfp.
This is preparation for removing the dependence of string_stream on
struct kunit, so that string_stream can be used for the debugfs log.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream-test.c | 26 +++++++++++++++-----------
lib/kunit/string-stream.c | 8 ++++----
lib/kunit/string-stream.h | 3 ++-
lib/kunit/test.c | 2 +-
4 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
index 46c2ac162fe8..8a30bb7d5fb7 100644
--- a/lib/kunit/string-stream-test.c
+++ b/lib/kunit/string-stream-test.c
@@ -57,7 +57,7 @@ static void string_stream_line_add_test(struct kunit *test)
}
num_lines = i;
- concat_string = string_stream_get_string(stream);
+ concat_string = string_stream_get_string(test, stream, GFP_KERNEL);
KUNIT_EXPECT_NOT_ERR_OR_NULL(test, concat_string);
KUNIT_EXPECT_EQ(test, strlen(concat_string), total_len);
@@ -113,7 +113,7 @@ static void string_stream_variable_length_line_test(struct kunit *test)
}
num_lines = i;
- concat_string = string_stream_get_string(stream);
+ concat_string = string_stream_get_string(test, stream, GFP_KERNEL);
KUNIT_EXPECT_NOT_ERR_OR_NULL(test, concat_string);
KUNIT_EXPECT_EQ(test, strlen(concat_string), total_len);
@@ -165,17 +165,18 @@ static void string_stream_append_test(struct kunit *test)
/* Append content of empty stream to empty stream */
string_stream_append(stream_1, stream_2);
- KUNIT_EXPECT_EQ(test, strlen(string_stream_get_string(stream_1)), 0);
+ KUNIT_EXPECT_EQ(test, strlen(string_stream_get_string(test, stream_1, GFP_KERNEL)), 0);
/* Add some data to stream_1 */
for (i = 0; i < ARRAY_SIZE(strings_1); ++i)
string_stream_add(stream_1, "%s\n", strings_1[i]);
- original_content = string_stream_get_string(stream_1);
+ original_content = string_stream_get_string(test, stream_1, GFP_KERNEL);
/* Append content of empty stream to non-empty stream */
string_stream_append(stream_1, stream_2);
- KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), original_content);
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(test, stream_1, GFP_KERNEL),
+ original_content);
/* Add some data to stream_2 */
for (i = 0; i < ARRAY_SIZE(strings_2); ++i)
@@ -188,14 +189,15 @@ static void string_stream_append_test(struct kunit *test)
* End result should be the original content of stream_1 plus
* the content of stream_2.
*/
- stream_2_content = string_stream_get_string(stream_2);
+ stream_2_content = string_stream_get_string(test, stream_2, GFP_KERNEL);
combined_length = strlen(original_content) + strlen(stream_2_content);
combined_length++; /* for terminating \0 */
combined_content = kunit_kmalloc(test, combined_length, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, combined_content);
snprintf(combined_content, combined_length, "%s%s", original_content, stream_2_content);
- KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), combined_content);
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(test, stream_1, GFP_KERNEL),
+ combined_content);
/* Append content of non-empty stream to empty stream */
string_stream_destroy(stream_1);
@@ -204,7 +206,8 @@ static void string_stream_append_test(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
string_stream_append(stream_1, stream_2);
- KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), stream_2_content);
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(test, stream_1, GFP_KERNEL),
+ stream_2_content);
}
/* Adding an empty string should not create a fragment. */
@@ -224,7 +227,8 @@ static void string_stream_append_empty_string_test(struct kunit *test)
string_stream_add(stream, "Add this line");
string_stream_add(stream, "%s", "");
KUNIT_EXPECT_EQ(test, list_count_nodes(&stream->fragments), 1);
- KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream), "Add this line");
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(test, stream, GFP_KERNEL),
+ "Add this line");
}
/* Adding strings without automatic newline appending */
@@ -244,7 +248,7 @@ static void string_stream_no_auto_newline_test(struct kunit *test)
string_stream_add(stream, "Two\n");
string_stream_add(stream, "%s\n", "Three");
string_stream_add(stream, "Four");
- KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream),
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(test, stream, GFP_KERNEL),
"OneTwo\nThree\nFour");
}
@@ -271,7 +275,7 @@ static void string_stream_auto_newline_test(struct kunit *test)
string_stream_add(stream, "Five\n%s", "Six");
string_stream_add(stream, "Seven\n\n");
string_stream_add(stream, "Eight");
- KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream),
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(test, stream, GFP_KERNEL),
"One\nTwo\nThree\nFour\nFive\nSix\nSeven\n\nEight\n");
}
diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
index 1dcf6513b692..eb673d3ea3bd 100644
--- a/lib/kunit/string-stream.c
+++ b/lib/kunit/string-stream.c
@@ -117,13 +117,14 @@ static void string_stream_clear(struct string_stream *stream)
spin_unlock(&stream->lock);
}
-char *string_stream_get_string(struct string_stream *stream)
+char *string_stream_get_string(struct kunit *test, struct string_stream *stream,
+ gfp_t gfp)
{
struct string_stream_fragment *frag_container;
size_t buf_len = stream->length + 1; /* +1 for null byte. */
char *buf;
- buf = kunit_kzalloc(stream->test, buf_len, stream->gfp);
+ buf = kunit_kzalloc(test, buf_len, gfp);
if (!buf)
return NULL;
@@ -140,8 +141,7 @@ int string_stream_append(struct string_stream *stream,
{
const char *other_content;
- other_content = string_stream_get_string(other);
-
+ other_content = string_stream_get_string(other->test, other, other->gfp);
if (!other_content)
return -ENOMEM;
diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h
index 048930bf97f0..6b4a747881c5 100644
--- a/lib/kunit/string-stream.h
+++ b/lib/kunit/string-stream.h
@@ -39,7 +39,8 @@ int __printf(2, 0) string_stream_vadd(struct string_stream *stream,
const char *fmt,
va_list args);
-char *string_stream_get_string(struct string_stream *stream);
+char *string_stream_get_string(struct kunit *test, struct string_stream *stream,
+ gfp_t gfp);
int string_stream_append(struct string_stream *stream,
struct string_stream *other);
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 49698a168437..520e15f49d0d 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -286,7 +286,7 @@ static void kunit_print_string_stream(struct kunit *test,
if (string_stream_is_empty(stream))
return;
- buf = string_stream_get_string(stream);
+ buf = string_stream_get_string(test, stream, GFP_KERNEL);
if (!buf) {
kunit_err(test,
"Could not allocate buffer, dumping stream:\n");
--
2.30.2
Replace the minimal tests with more-thorough testing.
string_stream_init_test() tests that struct string_stream is
initialized correctly.
string_stream_line_add_test() adds a series of numbered lines and
checks that the resulting string contains all the lines.
string_stream_variable_length_line_test() adds a large number of
lines of varying length to create many fragments, then tests that all
lines are present.
string_stream_append_test() tests various cases of using
string_stream_append() to append the content of one stream to another.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream-test.c | 200 ++++++++++++++++++++++++++++++---
1 file changed, 184 insertions(+), 16 deletions(-)
diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
index 110f3a993250..1d46d5f06d2a 100644
--- a/lib/kunit/string-stream-test.c
+++ b/lib/kunit/string-stream-test.c
@@ -11,38 +11,206 @@
#include "string-stream.h"
-static void string_stream_test_empty_on_creation(struct kunit *test)
+/* string_stream object is initialized correctly. */
+static void string_stream_init_test(struct kunit *test)
{
- struct string_stream *stream = alloc_string_stream(test, GFP_KERNEL);
+ struct string_stream *stream;
+
+ stream = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
+
+ KUNIT_EXPECT_EQ(test, stream->length, 0);
+ KUNIT_EXPECT_TRUE(test, list_empty(&stream->fragments));
+ KUNIT_EXPECT_PTR_EQ(test, stream->test, test);
+ KUNIT_EXPECT_EQ(test, stream->gfp, GFP_KERNEL);
KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
}
-static void string_stream_test_not_empty_after_add(struct kunit *test)
+/*
+ * Add a series of lines to a string_stream. Check that all lines
+ * appear in the correct order and no characters are dropped.
+ */
+static void string_stream_line_add_test(struct kunit *test)
{
- struct string_stream *stream = alloc_string_stream(test, GFP_KERNEL);
+ struct string_stream *stream;
+ char line[60];
+ char *concat_string, *pos, *string_end;
+ size_t len, total_len;
+ int num_lines, i;
- string_stream_add(stream, "Foo");
+ stream = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
- KUNIT_EXPECT_FALSE(test, string_stream_is_empty(stream));
+ /* Add series of sequence numbered lines */
+ total_len = 0;
+ for (i = 0; i < 100; ++i) {
+ len = snprintf(line, sizeof(line),
+ "The quick brown fox jumps over the lazy penguin %d\n", i);
+
+ /* Sanity-check that our test string isn't truncated */
+ KUNIT_ASSERT_LT(test, len, sizeof(line));
+
+ string_stream_add(stream, line);
+ total_len += len;
+ }
+ num_lines = i;
+
+ concat_string = string_stream_get_string(stream);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test, concat_string);
+ KUNIT_EXPECT_EQ(test, strlen(concat_string), total_len);
+
+ /*
+ * Split the concatenated string at the newlines and check that
+ * all the original added strings are present.
+ */
+ pos = concat_string;
+ for (i = 0; i < num_lines; ++i) {
+ string_end = strchr(pos, '\n');
+ KUNIT_EXPECT_NOT_NULL(test, string_end);
+
+ /* Convert to NULL-terminated string */
+ *string_end = '\0';
+
+ snprintf(line, sizeof(line),
+ "The quick brown fox jumps over the lazy penguin %d", i);
+ KUNIT_EXPECT_STREQ(test, pos, line);
+
+ pos = string_end + 1;
+ }
+
+ /* There shouldn't be any more data after this */
+ KUNIT_EXPECT_EQ(test, strlen(pos), 0);
}
-static void string_stream_test_get_string(struct kunit *test)
+/* Add a series of lines of variable length to a string_stream. */
+static void string_stream_variable_length_line_test(struct kunit *test)
{
- struct string_stream *stream = alloc_string_stream(test, GFP_KERNEL);
- char *output;
+ static const char line[] =
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ " 0123456789!$%^&*()_-+={}[]:;@'~#<>,.?/|";
+ struct string_stream *stream;
+ struct rnd_state rnd;
+ char *concat_string, *pos, *string_end;
+ size_t offset, total_len;
+ int num_lines, i;
- string_stream_add(stream, "Foo");
- string_stream_add(stream, " %s", "bar");
+ stream = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
- output = string_stream_get_string(stream);
- KUNIT_ASSERT_STREQ(test, output, "Foo bar");
+ /*
+ * Log many lines of varying lengths until we have created
+ * many fragments.
+ * The "randomness" must be repeatable.
+ */
+ prandom_seed_state(&rnd, 3141592653589793238ULL);
+ total_len = 0;
+ for (i = 0; i < 100; ++i) {
+ offset = prandom_u32_state(&rnd) % (sizeof(line) - 1);
+ string_stream_add(stream, "%s\n", &line[offset]);
+ total_len += sizeof(line) - offset;
+ }
+ num_lines = i;
+
+ concat_string = string_stream_get_string(stream);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test, concat_string);
+ KUNIT_EXPECT_EQ(test, strlen(concat_string), total_len);
+
+ /*
+ * Split the concatenated string at the newlines and check that
+ * all the original added strings are present.
+ */
+ prandom_seed_state(&rnd, 3141592653589793238ULL);
+ pos = concat_string;
+ for (i = 0; i < num_lines; ++i) {
+ string_end = strchr(pos, '\n');
+ KUNIT_EXPECT_NOT_NULL(test, string_end);
+
+ /* Convert to NULL-terminated string */
+ *string_end = '\0';
+
+ offset = prandom_u32_state(&rnd) % (sizeof(line) - 1);
+ KUNIT_EXPECT_STREQ(test, pos, &line[offset]);
+
+ pos = string_end + 1;
+ }
+
+ /* There shouldn't be any more data after this */
+ KUNIT_EXPECT_EQ(test, strlen(pos), 0);
+}
+
+/* Appending the content of one string stream to another. */
+static void string_stream_append_test(struct kunit *test)
+{
+ static const char * const strings_1[] = {
+ "one", "two", "three", "four", "five", "six",
+ "seven", "eight", "nine", "ten",
+ };
+ static const char * const strings_2[] = {
+ "ONE", "TWO", "THREE", "FOUR", "FIVE", "SIZE",
+ "SEVEN", "EIGHT", "NINE", "TEN",
+ };
+ struct string_stream *stream_1, *stream_2;
+ const char *original_content, *stream_2_content;
+ char *combined_content;
+ size_t combined_length;
+ int i;
+
+ stream_1 = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
+
+ stream_2 = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_2);
+
+ /* Append content of empty stream to empty stream */
+ string_stream_append(stream_1, stream_2);
+ KUNIT_EXPECT_EQ(test, strlen(string_stream_get_string(stream_1)), 0);
+
+ /* Add some data to stream_1 */
+ for (i = 0; i < ARRAY_SIZE(strings_1); ++i)
+ string_stream_add(stream_1, "%s\n", strings_1[i]);
+
+ original_content = string_stream_get_string(stream_1);
+
+ /* Append content of empty stream to non-empty stream */
+ string_stream_append(stream_1, stream_2);
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), original_content);
+
+ /* Add some data to stream_2 */
+ for (i = 0; i < ARRAY_SIZE(strings_2); ++i)
+ string_stream_add(stream_2, "%s\n", strings_2[i]);
+
+ /* Append content of non-empty stream to non-empty stream */
+ string_stream_append(stream_1, stream_2);
+
+ /*
+ * End result should be the original content of stream_1 plus
+ * the content of stream_2.
+ */
+ stream_2_content = string_stream_get_string(stream_2);
+ combined_length = strlen(original_content) + strlen(stream_2_content);
+ combined_length++; /* for terminating \0 */
+ combined_content = kunit_kmalloc(test, combined_length, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, combined_content);
+ snprintf(combined_content, combined_length, "%s%s", original_content, stream_2_content);
+
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), combined_content);
+
+ /* Append content of non-empty stream to empty stream */
+ string_stream_destroy(stream_1);
+
+ stream_1 = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
+
+ string_stream_append(stream_1, stream_2);
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), stream_2_content);
}
static struct kunit_case string_stream_test_cases[] = {
- KUNIT_CASE(string_stream_test_empty_on_creation),
- KUNIT_CASE(string_stream_test_not_empty_after_add),
- KUNIT_CASE(string_stream_test_get_string),
+ KUNIT_CASE(string_stream_init_test),
+ KUNIT_CASE(string_stream_line_add_test),
+ KUNIT_CASE(string_stream_variable_length_line_test),
+ KUNIT_CASE(string_stream_append_test),
{}
};
--
2.30.2
Re-work string_stream so that it is not tied to a struct kunit. This is
to allow using it for the log of struct kunit_suite.
Instead of resource-managing individual allocations the whole string_stream
object can be resource-managed as a single object:
alloc_string_stream() API is unchanged and takes a pointer to a
struct kunit but it now registers the returned string_stream object to
be resource-managed.
raw_alloc_string_stream() is a new function that allocates a
bare string_stream without any association to a struct kunit.
free_string_stream() is a new function that frees a resource-managed
string_stream allocated by alloc_string_stream().
raw_free_string_stream() is a new function that frees a non-managed
string_stream allocated by raw_alloc_string_stream().
The confusing function string_stream_destroy() has been removed. This only
called string_stream_clear() but didn't free the struct string_stream.
Instead string_stream_clear() has been exported, and the new functions use
the more conventional naming of "free" as the opposite of "alloc".
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream-test.c | 2 +-
lib/kunit/string-stream.c | 92 +++++++++++++++++++++++-----------
lib/kunit/string-stream.h | 12 ++++-
lib/kunit/test.c | 2 +-
4 files changed, 77 insertions(+), 31 deletions(-)
diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
index 8a30bb7d5fb7..437aa4b3179d 100644
--- a/lib/kunit/string-stream-test.c
+++ b/lib/kunit/string-stream-test.c
@@ -200,7 +200,7 @@ static void string_stream_append_test(struct kunit *test)
combined_content);
/* Append content of non-empty stream to empty stream */
- string_stream_destroy(stream_1);
+ string_stream_clear(stream_1);
stream_1 = alloc_string_stream(test, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream_1);
diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c
index eb673d3ea3bd..06104a729b45 100644
--- a/lib/kunit/string-stream.c
+++ b/lib/kunit/string-stream.c
@@ -13,30 +13,28 @@
#include "string-stream.h"
-static struct string_stream_fragment *alloc_string_stream_fragment(
- struct kunit *test, int len, gfp_t gfp)
+static struct string_stream_fragment *alloc_string_stream_fragment(int len, gfp_t gfp)
{
struct string_stream_fragment *frag;
- frag = kunit_kzalloc(test, sizeof(*frag), gfp);
+ frag = kzalloc(sizeof(*frag), gfp);
if (!frag)
return ERR_PTR(-ENOMEM);
- frag->fragment = kunit_kmalloc(test, len, gfp);
+ frag->fragment = kmalloc(len, gfp);
if (!frag->fragment) {
- kunit_kfree(test, frag);
+ kfree(frag);
return ERR_PTR(-ENOMEM);
}
return frag;
}
-static void string_stream_fragment_destroy(struct kunit *test,
- struct string_stream_fragment *frag)
+static void string_stream_fragment_destroy(struct string_stream_fragment *frag)
{
list_del(&frag->node);
- kunit_kfree(test, frag->fragment);
- kunit_kfree(test, frag);
+ kfree(frag->fragment);
+ kfree(frag);
}
int string_stream_vadd(struct string_stream *stream,
@@ -65,9 +63,7 @@ int string_stream_vadd(struct string_stream *stream,
/* Need space for null byte. */
buf_len++;
- frag_container = alloc_string_stream_fragment(stream->test,
- buf_len,
- stream->gfp);
+ frag_container = alloc_string_stream_fragment(buf_len, stream->gfp);
if (IS_ERR(frag_container))
return PTR_ERR(frag_container);
@@ -102,7 +98,7 @@ int string_stream_add(struct string_stream *stream, const char *fmt, ...)
return result;
}
-static void string_stream_clear(struct string_stream *stream)
+void string_stream_clear(struct string_stream *stream)
{
struct string_stream_fragment *frag_container, *frag_container_safe;
@@ -111,16 +107,28 @@ static void string_stream_clear(struct string_stream *stream)
frag_container_safe,
&stream->fragments,
node) {
- string_stream_fragment_destroy(stream->test, frag_container);
+ string_stream_fragment_destroy(frag_container);
}
stream->length = 0;
spin_unlock(&stream->lock);
}
+static void _string_stream_concatenate_to_buf(struct string_stream *stream,
+ char *buf, size_t buf_len)
+{
+ struct string_stream_fragment *frag_container;
+
+ buf[0] = '\0';
+
+ spin_lock(&stream->lock);
+ list_for_each_entry(frag_container, &stream->fragments, node)
+ strlcat(buf, frag_container->fragment, buf_len);
+ spin_unlock(&stream->lock);
+}
+
char *string_stream_get_string(struct kunit *test, struct string_stream *stream,
gfp_t gfp)
{
- struct string_stream_fragment *frag_container;
size_t buf_len = stream->length + 1; /* +1 for null byte. */
char *buf;
@@ -128,10 +136,7 @@ char *string_stream_get_string(struct kunit *test, struct string_stream *stream,
if (!buf)
return NULL;
- spin_lock(&stream->lock);
- list_for_each_entry(frag_container, &stream->fragments, node)
- strlcat(buf, frag_container->fragment, buf_len);
- spin_unlock(&stream->lock);
+ _string_stream_concatenate_to_buf(stream, buf, buf_len);
return buf;
}
@@ -139,13 +144,20 @@ char *string_stream_get_string(struct kunit *test, struct string_stream *stream,
int string_stream_append(struct string_stream *stream,
struct string_stream *other)
{
- const char *other_content;
+ size_t other_buf_len = other->length + 1; /* +1 for null byte. */
+ char *other_buf;
+ int ret;
- other_content = string_stream_get_string(other->test, other, other->gfp);
- if (!other_content)
+ other_buf = kmalloc(other_buf_len, GFP_KERNEL);
+ if (!other_buf)
return -ENOMEM;
- return string_stream_add(stream, other_content);
+ _string_stream_concatenate_to_buf(other, other_buf, other_buf_len);
+
+ ret = string_stream_add(stream, other_buf);
+ kfree(other_buf);
+
+ return ret;
}
bool string_stream_is_empty(struct string_stream *stream)
@@ -153,23 +165,47 @@ bool string_stream_is_empty(struct string_stream *stream)
return list_empty(&stream->fragments);
}
-struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp)
+void raw_free_string_stream(struct string_stream *stream)
+{
+ string_stream_clear(stream);
+ kfree(stream);
+}
+
+struct string_stream *raw_alloc_string_stream(gfp_t gfp)
{
struct string_stream *stream;
- stream = kunit_kzalloc(test, sizeof(*stream), gfp);
+ stream = kzalloc(sizeof(*stream), gfp);
if (!stream)
return ERR_PTR(-ENOMEM);
stream->gfp = gfp;
- stream->test = test;
INIT_LIST_HEAD(&stream->fragments);
spin_lock_init(&stream->lock);
return stream;
}
-void string_stream_destroy(struct string_stream *stream)
+struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp)
{
- string_stream_clear(stream);
+ struct string_stream *stream;
+
+ stream = raw_alloc_string_stream(gfp);
+ if (IS_ERR(stream))
+ return stream;
+
+ stream->test = test;
+
+ if (kunit_add_action_or_reset(test, (kunit_action_t *)raw_free_string_stream, stream) != 0)
+ return ERR_PTR(-ENOMEM);
+
+ return stream;
+}
+
+void free_string_stream(struct kunit *test, struct string_stream *stream)
+{
+ if (!stream)
+ return;
+
+ kunit_release_action(test, (kunit_action_t *)raw_free_string_stream, (void *)stream);
}
diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h
index 6b4a747881c5..fbeda486382a 100644
--- a/lib/kunit/string-stream.h
+++ b/lib/kunit/string-stream.h
@@ -23,14 +23,24 @@ struct string_stream {
struct list_head fragments;
/* length and fragments are protected by this lock */
spinlock_t lock;
+
+ /*
+ * Pointer to kunit this stream is associated with, or NULL if
+ * not associated with a kunit.
+ */
struct kunit *test;
+
gfp_t gfp;
bool append_newlines;
};
struct kunit;
+struct string_stream *raw_alloc_string_stream(gfp_t gfp);
+void raw_free_string_stream(struct string_stream *stream);
+
struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp);
+void free_string_stream(struct kunit *test, struct string_stream *stream);
int __printf(2, 3) string_stream_add(struct string_stream *stream,
const char *fmt, ...);
@@ -47,7 +57,7 @@ int string_stream_append(struct string_stream *stream,
bool string_stream_is_empty(struct string_stream *stream);
-void string_stream_destroy(struct string_stream *stream);
+void string_stream_clear(struct string_stream *stream);
static inline void string_stream_set_append_newlines(struct string_stream *stream,
bool append_newlines)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 520e15f49d0d..4b69d12dfc96 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -322,7 +322,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc,
kunit_print_string_stream(test, stream);
- string_stream_destroy(stream);
+ free_string_stream(test, stream);
}
void __noreturn __kunit_abort(struct kunit *test)
--
2.30.2
Add test cases for testing the string_stream feature that appends a
newline to strings that do not already end with a newline.
string_stream_no_auto_newline_test() tests with this feature disabled.
Newlines should not be added or dropped.
string_stream_auto_newline_test() tests with this feature enabled.
Newlines should be added to lines that do not end with a newline.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
lib/kunit/string-stream-test.c | 51 ++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
index efe13e3322b5..46c2ac162fe8 100644
--- a/lib/kunit/string-stream-test.c
+++ b/lib/kunit/string-stream-test.c
@@ -23,6 +23,7 @@ static void string_stream_init_test(struct kunit *test)
KUNIT_EXPECT_TRUE(test, list_empty(&stream->fragments));
KUNIT_EXPECT_PTR_EQ(test, stream->test, test);
KUNIT_EXPECT_EQ(test, stream->gfp, GFP_KERNEL);
+ KUNIT_EXPECT_FALSE(test, stream->append_newlines);
KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
}
@@ -226,12 +227,62 @@ static void string_stream_append_empty_string_test(struct kunit *test)
KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream), "Add this line");
}
+/* Adding strings without automatic newline appending */
+static void string_stream_no_auto_newline_test(struct kunit *test)
+{
+ struct string_stream *stream;
+
+ stream = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
+
+ /*
+ * Add some strings with and without newlines. All formatted
+ * newlines should be preserved. No extra newlines should be
+ * added.
+ */
+ string_stream_add(stream, "One");
+ string_stream_add(stream, "Two\n");
+ string_stream_add(stream, "%s\n", "Three");
+ string_stream_add(stream, "Four");
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream),
+ "OneTwo\nThree\nFour");
+}
+
+/* Adding strings with automatic newline appending */
+static void string_stream_auto_newline_test(struct kunit *test)
+{
+ struct string_stream *stream;
+
+ stream = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
+
+ string_stream_set_append_newlines(stream, true);
+ KUNIT_EXPECT_TRUE(test, stream->append_newlines);
+
+ /*
+ * Add some strings with and without newlines. Newlines should
+ * be appended to lines that do not end with \n, but newlines
+ * resulting from the formatting should not be changed.
+ */
+ string_stream_add(stream, "One");
+ string_stream_add(stream, "Two\n");
+ string_stream_add(stream, "%s\n", "Three");
+ string_stream_add(stream, "%s", "Four\n");
+ string_stream_add(stream, "Five\n%s", "Six");
+ string_stream_add(stream, "Seven\n\n");
+ string_stream_add(stream, "Eight");
+ KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream),
+ "One\nTwo\nThree\nFour\nFive\nSix\nSeven\n\nEight\n");
+}
+
static struct kunit_case string_stream_test_cases[] = {
KUNIT_CASE(string_stream_init_test),
KUNIT_CASE(string_stream_line_add_test),
KUNIT_CASE(string_stream_variable_length_line_test),
KUNIT_CASE(string_stream_append_test),
KUNIT_CASE(string_stream_append_empty_string_test),
+ KUNIT_CASE(string_stream_no_auto_newline_test),
+ KUNIT_CASE(string_stream_auto_newline_test),
{}
};
--
2.30.2
Replace the fixed-size log buffer with a string_stream so that the
log can grow as lines are added.
The existing kunit log tests have been updated for using a
string_stream as the log. As they now depend on string_stream
functions they cannot build when kunit-test is a module. They have
been moved to a separate source file to be built only if kunit-test
is built-in.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
include/kunit/test.h | 14 ++++----
lib/kunit/Makefile | 5 +--
lib/kunit/debugfs.c | 36 +++++++++++++--------
lib/kunit/kunit-test.c | 52 +-----------------------------
lib/kunit/log-test.c | 72 ++++++++++++++++++++++++++++++++++++++++++
lib/kunit/test.c | 44 +++-----------------------
6 files changed, 110 insertions(+), 113 deletions(-)
create mode 100644 lib/kunit/log-test.c
diff --git a/include/kunit/test.h b/include/kunit/test.h
index d33114097d0d..b915a0fe93c0 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -32,9 +32,7 @@
DECLARE_STATIC_KEY_FALSE(kunit_running);
struct kunit;
-
-/* Size of log associated with test. */
-#define KUNIT_LOG_SIZE 2048
+struct string_stream;
/* Maximum size of parameter description string. */
#define KUNIT_PARAM_DESC_SIZE 128
@@ -132,7 +130,7 @@ struct kunit_case {
/* private: internal use only. */
enum kunit_status status;
char *module_name;
- char *log;
+ struct string_stream *log;
};
static inline char *kunit_status_to_ok_not_ok(enum kunit_status status)
@@ -252,7 +250,7 @@ struct kunit_suite {
/* private: internal use only */
char status_comment[KUNIT_STATUS_COMMENT_SIZE];
struct dentry *debugfs;
- char *log;
+ struct string_stream *log;
int suite_init_err;
};
@@ -278,7 +276,7 @@ struct kunit {
/* private: internal use only. */
const char *name; /* Read only after initialization! */
- char *log; /* Points at case log after initialization */
+ struct string_stream *log; /* Points at case log after initialization */
struct kunit_try_catch try_catch;
/* param_value is the current parameter value for a test case. */
const void *param_value;
@@ -314,7 +312,7 @@ const char *kunit_filter_glob(void);
char *kunit_filter(void);
char *kunit_filter_action(void);
-void kunit_init_test(struct kunit *test, const char *name, char *log);
+void kunit_init_test(struct kunit *test, const char *name, struct string_stream *log);
int kunit_run_tests(struct kunit_suite *suite);
@@ -472,7 +470,7 @@ static inline void *kunit_kcalloc(struct kunit *test, size_t n, size_t size, gfp
void kunit_cleanup(struct kunit *test);
-void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...);
+void __printf(2, 3) kunit_log_append(struct string_stream *log, const char *fmt, ...);
/**
* kunit_mark_skipped() - Marks @test_or_suite as skipped
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 46f75f23dfe4..65735c2e1d14 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -18,9 +18,10 @@ obj-y += hooks.o
obj-$(CONFIG_KUNIT_TEST) += kunit-test.o
-# string-stream-test compiles built-in only.
+# string-stream-test and log-test compiles built-in only.
ifeq ($(CONFIG_KUNIT_TEST),y)
-obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
+obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o \
+ log-test.o
endif
obj-$(CONFIG_KUNIT_EXAMPLE_TEST) += kunit-example-test.o
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 22c5c496a68f..ab53a7e5c898 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -37,14 +37,21 @@ void kunit_debugfs_init(void)
debugfs_rootdir = debugfs_create_dir(KUNIT_DEBUGFS_ROOT, NULL);
}
-static void debugfs_print_result(struct seq_file *seq,
- struct kunit_suite *suite,
- struct kunit_case *test_case)
+static void debugfs_print_result(struct seq_file *seq, struct string_stream *log)
{
- if (!test_case || !test_case->log)
+ struct string_stream_fragment *frag_container;
+
+ if (!log)
return;
- seq_printf(seq, "%s", test_case->log);
+ /*
+ * Walk the fragments so we don't need to allocate a temporary
+ * buffer to hold the entire string.
+ */
+ spin_lock(&log->lock);
+ list_for_each_entry(frag_container, &log->fragments, node)
+ seq_printf(seq, "%s", frag_container->fragment);
+ spin_unlock(&log->lock);
}
/*
@@ -69,10 +76,9 @@ static int debugfs_print_results(struct seq_file *seq, void *v)
seq_printf(seq, KUNIT_SUBTEST_INDENT "1..%zd\n", kunit_suite_num_test_cases(suite));
kunit_suite_for_each_test_case(suite, test_case)
- debugfs_print_result(seq, suite, test_case);
+ debugfs_print_result(seq, test_case->log);
- if (suite->log)
- seq_printf(seq, "%s", suite->log);
+ debugfs_print_result(seq, suite->log);
seq_printf(seq, "%s %d %s\n",
kunit_status_to_ok_not_ok(success), 1, suite->name);
@@ -105,9 +111,13 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
struct kunit_case *test_case;
/* Allocate logs before creating debugfs representation. */
- suite->log = kzalloc(KUNIT_LOG_SIZE, GFP_KERNEL);
- kunit_suite_for_each_test_case(suite, test_case)
- test_case->log = kzalloc(KUNIT_LOG_SIZE, GFP_KERNEL);
+ suite->log = raw_alloc_string_stream(GFP_KERNEL);
+ string_stream_set_append_newlines(suite->log, true);
+
+ kunit_suite_for_each_test_case(suite, test_case) {
+ test_case->log = raw_alloc_string_stream(GFP_KERNEL);
+ string_stream_set_append_newlines(test_case->log, true);
+ }
suite->debugfs = debugfs_create_dir(suite->name, debugfs_rootdir);
@@ -121,7 +131,7 @@ void kunit_debugfs_destroy_suite(struct kunit_suite *suite)
struct kunit_case *test_case;
debugfs_remove_recursive(suite->debugfs);
- kfree(suite->log);
+ raw_free_string_stream(suite->log);
kunit_suite_for_each_test_case(suite, test_case)
- kfree(test_case->log);
+ raw_free_string_stream(test_case->log);
}
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
index 83d8e90ca7a2..ecc39d5f7411 100644
--- a/lib/kunit/kunit-test.c
+++ b/lib/kunit/kunit-test.c
@@ -530,55 +530,6 @@ static struct kunit_suite kunit_resource_test_suite = {
.test_cases = kunit_resource_test_cases,
};
-static void kunit_log_test(struct kunit *test)
-{
- struct kunit_suite suite;
-
- suite.log = kunit_kzalloc(test, KUNIT_LOG_SIZE, GFP_KERNEL);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log);
-
- kunit_log(KERN_INFO, test, "put this in log.");
- kunit_log(KERN_INFO, test, "this too.");
- kunit_log(KERN_INFO, &suite, "add to suite log.");
- kunit_log(KERN_INFO, &suite, "along with this.");
-
-#ifdef CONFIG_KUNIT_DEBUGFS
- KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
- strstr(test->log, "put this in log."));
- KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
- strstr(test->log, "this too."));
- KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
- strstr(suite.log, "add to suite log."));
- KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
- strstr(suite.log, "along with this."));
-#else
- KUNIT_EXPECT_NULL(test, test->log);
-#endif
-}
-
-static void kunit_log_newline_test(struct kunit *test)
-{
- kunit_info(test, "Add newline\n");
- if (test->log) {
- KUNIT_ASSERT_NOT_NULL_MSG(test, strstr(test->log, "Add newline\n"),
- "Missing log line, full log:\n%s", test->log);
- KUNIT_EXPECT_NULL(test, strstr(test->log, "Add newline\n\n"));
- } else {
- kunit_skip(test, "only useful when debugfs is enabled");
- }
-}
-
-static struct kunit_case kunit_log_test_cases[] = {
- KUNIT_CASE(kunit_log_test),
- KUNIT_CASE(kunit_log_newline_test),
- {}
-};
-
-static struct kunit_suite kunit_log_test_suite = {
- .name = "kunit-log-test",
- .test_cases = kunit_log_test_cases,
-};
-
static void kunit_status_set_failure_test(struct kunit *test)
{
struct kunit fake;
@@ -658,7 +609,6 @@ static struct kunit_suite kunit_current_test_suite = {
};
kunit_test_suites(&kunit_try_catch_test_suite, &kunit_resource_test_suite,
- &kunit_log_test_suite, &kunit_status_test_suite,
- &kunit_current_test_suite);
+ &kunit_status_test_suite, &kunit_current_test_suite);
MODULE_LICENSE("GPL v2");
diff --git a/lib/kunit/log-test.c b/lib/kunit/log-test.c
new file mode 100644
index 000000000000..a93d87112fea
--- /dev/null
+++ b/lib/kunit/log-test.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for logging.
+ *
+ * Based on code:
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <[email protected]>
+ */
+#include <kunit/test.h>
+
+#include "string-stream.h"
+
+static void kunit_log_test(struct kunit *test)
+{
+ struct kunit_suite suite;
+ char *full_log;
+
+ suite.log = alloc_string_stream(test, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log);
+ string_stream_set_append_newlines(suite.log, true);
+
+ kunit_log(KERN_INFO, test, "put this in log.");
+ kunit_log(KERN_INFO, test, "this too.");
+ kunit_log(KERN_INFO, &suite, "add to suite log.");
+ kunit_log(KERN_INFO, &suite, "along with this.");
+
+#ifdef CONFIG_KUNIT_DEBUGFS
+ KUNIT_EXPECT_TRUE(test, test->log->append_newlines);
+
+ full_log = string_stream_get_string(test, test->log, GFP_KERNEL);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
+ strstr(full_log, "put this in log."));
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
+ strstr(full_log, "this too."));
+
+ full_log = string_stream_get_string(test, suite.log, GFP_KERNEL);
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
+ strstr(full_log, "add to suite log."));
+ KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
+ strstr(full_log, "along with this."));
+#else
+ KUNIT_EXPECT_NULL(test, test->log);
+#endif
+}
+
+static void kunit_log_newline_test(struct kunit *test)
+{
+ char *full_log;
+
+ kunit_info(test, "Add newline\n");
+ if (test->log) {
+ full_log = string_stream_get_string(test, test->log, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL_MSG(test, strstr(full_log, "Add newline\n"),
+ "Missing log line, full log:\n%s", full_log);
+ KUNIT_EXPECT_NULL(test, strstr(full_log, "Add newline\n\n"));
+ } else {
+ kunit_skip(test, "only useful when debugfs is enabled");
+ }
+}
+
+static struct kunit_case kunit_log_test_cases[] = {
+ KUNIT_CASE(kunit_log_test),
+ KUNIT_CASE(kunit_log_newline_test),
+ {}
+};
+
+static struct kunit_suite kunit_log_test_suite = {
+ .name = "kunit-log-test",
+ .test_cases = kunit_log_test_cases,
+};
+
+kunit_test_suites(&kunit_log_test_suite);
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 4b69d12dfc96..14e889e80129 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -109,51 +109,17 @@ static void kunit_print_test_stats(struct kunit *test,
stats.total);
}
-/**
- * kunit_log_newline() - Add newline to the end of log if one is not
- * already present.
- * @log: The log to add the newline to.
- */
-static void kunit_log_newline(char *log)
-{
- int log_len, len_left;
-
- log_len = strlen(log);
- len_left = KUNIT_LOG_SIZE - log_len - 1;
-
- if (log_len > 0 && log[log_len - 1] != '\n')
- strncat(log, "\n", len_left);
-}
-
-/*
- * Append formatted message to log, size of which is limited to
- * KUNIT_LOG_SIZE bytes (including null terminating byte).
- */
-void kunit_log_append(char *log, const char *fmt, ...)
+/* Append formatted message to log. */
+void kunit_log_append(struct string_stream *log, const char *fmt, ...)
{
va_list args;
- int len, log_len, len_left;
if (!log)
return;
- log_len = strlen(log);
- len_left = KUNIT_LOG_SIZE - log_len - 1;
- if (len_left <= 0)
- return;
-
- /* Evaluate length of line to add to log */
va_start(args, fmt);
- len = vsnprintf(NULL, 0, fmt, args) + 1;
+ string_stream_vadd(log, fmt, args);
va_end(args);
-
- /* Print formatted line to the log */
- va_start(args, fmt);
- vsnprintf(log + log_len, min(len, len_left), fmt, args);
- va_end(args);
-
- /* Add newline to end of log if not already present. */
- kunit_log_newline(log);
}
EXPORT_SYMBOL_GPL(kunit_log_append);
@@ -359,14 +325,14 @@ void __kunit_do_failed_assertion(struct kunit *test,
}
EXPORT_SYMBOL_GPL(__kunit_do_failed_assertion);
-void kunit_init_test(struct kunit *test, const char *name, char *log)
+void kunit_init_test(struct kunit *test, const char *name, struct string_stream *log)
{
spin_lock_init(&test->lock);
INIT_LIST_HEAD(&test->resources);
test->name = name;
test->log = log;
if (test->log)
- test->log[0] = '\0';
+ string_stream_clear(log);
test->status = KUNIT_SUCCESS;
test->status_comment[0] = '\0';
}
--
2.30.2
On Mon, Aug 14, 2023 at 9:23 AM Richard Fitzgerald
<[email protected]> wrote:
>
> This patch chain changes the logging implementation to use string_stream
> so that the log will grow dynamically.
>
> The first 8 patches add test code for string_stream, and make some
> changes to string_stream needed to be able to use it for the log.
>
> The final patch adds a performance report of string_stream.
>
> CHANGES SINCE V3:
>
> Completely rewritten to use string_stream instead of implementing a
> separate extending-buffer implementation for logging.
>
> I have used the performance test from the final patch on my original
> fixed-size-fragment implementation from V3 to get a comparison of the
> two implementations (run on i3-8145UE CPU @ 2.20GHz):
>
> string_stream V3 fixed-size-buffer
> Time elapsed: 7748 us 3251 us
> Total string length: 573890 573890
> Bytes requested: 823994 728336
> Actual bytes allocated: 1061440 728352
>
> I don't think the difference is enough to be worth complicating the
> string_stream implementation with my fixed-fragment implementation from
> V3 of this patch chain.
Hello!
I just wanted to note that I have tested this series and it looks good
to me. I specifically looked over the newline handling and that looks
really good.
I understand you will likely be doing a new version of this. But other
than the things noted in comments by David, this seems to be working
really well.
Tested-by: Rae Moar <[email protected]>
Thanks for all the work you did in moving this framework to string-stream!
-Rae
>
> Richard Fitzgerald (10):
> kunit: string-stream: Improve testing of string_stream
> kunit: string-stream: Don't create a fragment for empty strings
> kunit: string-stream: Add cases for adding empty strings to a
> string_stream
> kunit: string-stream: Add option to make all lines end with newline
> kunit: string-stream: Add cases for string_stream newline appending
> kunit: string-stream: Pass struct kunit to string_stream_get_string()
> kunit: string-stream: Decouple string_stream from kunit
> kunit: string-stream: Add test for freeing resource-managed
> string_stream
> kunit: Use string_stream for test log
> kunit: string-stream: Test performance of string_stream
>
> include/kunit/test.h | 14 +-
> lib/kunit/Makefile | 5 +-
> lib/kunit/debugfs.c | 36 ++-
> lib/kunit/kunit-test.c | 52 +---
> lib/kunit/log-test.c | 72 ++++++
> lib/kunit/string-stream-test.c | 447 +++++++++++++++++++++++++++++++--
> lib/kunit/string-stream.c | 129 +++++++---
> lib/kunit/string-stream.h | 22 +-
> lib/kunit/test.c | 48 +---
> 9 files changed, 656 insertions(+), 169 deletions(-)
> create mode 100644 lib/kunit/log-test.c
>
> --
> 2.30.2
>