Quoting Brendan Higgins (2019-07-12 01:17:30)
diff --git a/include/kunit/kunit-stream.h b/include/kunit/kunit-stream.h new file mode 100644 index 0000000000000..a7b53eabf6be4 --- /dev/null +++ b/include/kunit/kunit-stream.h @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- C++ stream style string formatter and printer used in KUnit for outputting
- KUnit messages.
- Copyright (C) 2019, Google LLC.
- Author: Brendan Higgins brendanhiggins@google.com
- */
+#ifndef _KUNIT_KUNIT_STREAM_H +#define _KUNIT_KUNIT_STREAM_H
+#include <linux/types.h> +#include <kunit/string-stream.h>
+struct kunit;
+/**
- struct kunit_stream - a std::stream style string builder.
- A std::stream style string builder. Allows messages to be built up and
- printed all at once.
- */
+struct kunit_stream {
/* private: internal use only. */struct kunit *test;const char *level;
Is the level changed? See my comment below, but I wonder if this whole struct can go away and the wrappers can just operate on 'struct string_stream' instead.
struct string_stream *internal_stream;+}; diff --git a/kunit/kunit-stream.c b/kunit/kunit-stream.c new file mode 100644 index 0000000000000..8bea1f22eafb5 --- /dev/null +++ b/kunit/kunit-stream.c @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- C++ stream style string formatter and printer used in KUnit for outputting
- KUnit messages.
- Copyright (C) 2019, Google LLC.
- Author: Brendan Higgins brendanhiggins@google.com
- */
+#include <kunit/test.h> +#include <kunit/kunit-stream.h> +#include <kunit/string-stream.h>
+void kunit_stream_add(struct kunit_stream *kstream, const char *fmt, ...) +{
va_list args;struct string_stream *stream = kstream->internal_stream;va_start(args, fmt);if (string_stream_vadd(stream, fmt, args) < 0)kunit_err(kstream->test,"Failed to allocate fragment: %s\n",fmt);va_end(args);+}
+void kunit_stream_append(struct kunit_stream *kstream,
struct kunit_stream *other)+{
struct string_stream *other_stream = other->internal_stream;const char *other_content;other_content = string_stream_get_string(other_stream);if (!other_content) {kunit_err(kstream->test,"Failed to get string from second argument for appending\n");return;}kunit_stream_add(kstream, other_content);+}
Why can't this function be implemented in the string_stream API? Seems valid to want to append one stream to another and that isn't kunit_stream specific.
+void kunit_stream_clear(struct kunit_stream *kstream) +{
string_stream_clear(kstream->internal_stream);+}
+void kunit_stream_commit(struct kunit_stream *kstream) +{
struct string_stream *stream = kstream->internal_stream;struct string_stream_fragment *fragment;struct kunit *test = kstream->test;char *buf;buf = string_stream_get_string(stream);if (!buf) {kunit_err(test,"Could not allocate buffer, dumping stream:\n");list_for_each_entry(fragment, &stream->fragments, node) {kunit_err(test, fragment->fragment);}kunit_err(test, "\n");goto cleanup;}kunit_printk(kstream->level, test, buf);kfree(buf);+cleanup:
Drop the goto and use an 'else' please.
kunit_stream_clear(kstream);+}
+static int kunit_stream_init(struct kunit_resource *res, void *context) +{
struct kunit *test = context;struct kunit_stream *stream;stream = kzalloc(sizeof(*stream), GFP_KERNEL);if (!stream)return -ENOMEM;res->allocation = stream;stream->test = test;stream->internal_stream = alloc_string_stream(test);if (!stream->internal_stream)return -ENOMEM;return 0;+}
+static void kunit_stream_free(struct kunit_resource *res) +{
struct kunit_stream *stream = res->allocation;if (!string_stream_is_empty(stream->internal_stream)) {kunit_err(stream->test,"End of test case reached with uncommitted stream entries\n");kunit_stream_commit(stream);}+}
Nitpick: Drop this extra newline.
diff --git a/kunit/test.c b/kunit/test.c index f165c9d8e10b0..29edf34a89a37 100644 --- a/kunit/test.c +++ b/kunit/test.c @@ -120,6 +120,12 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case, test_case->name); } +void kunit_fail(struct kunit *test, struct kunit_stream *stream)
Why doesn't 'struct kunit' have a 'struct kunit_stream' inside of it? It seems that the two are highly related, to the point that it might just make sense to have
struct kunit { struct kunit_stream stream; ... };
+{
kunit_set_failure(test);kunit_stream_commit(stream);
And then this function can just take a test and the stream can be associated with the test directly. Use container_of() to get to the test when the only pointer in hand is for the stream too.
+}
void kunit_init_test(struct kunit *test, const char *name) { mutex_init(&test->lock);