| Message ID | 20230814132309.32641-7-rf@opensource.cirrus.com |
|---|---|
| State | New |
| Headers | show |
| Series | kunit: Add dynamically-extending log | expand |
On 15/8/23 10:16, David Gow wrote: > On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> >> 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 <rf@opensource.cirrus.com> >> --- > > Makes sense to me. > > I think that, if we weren't going to remove the struct kunit > dependency, we'd want to either keep a version of > string_stream_get_string() which uses it, or, e.g., fall back to it if > the struct passed in is NULL. > That was my first thought. But I thought that was open to subtle accidental bugs in calling code - it might return you a managed allocation, or it might return you an unmanaged allocation that you must free. As there weren't many callers of string_stream_get_string() I decided to go with changing the API to pass in test and gfp like other managed allocations. This makes it more generalized, since the returned buffer is not part of the stream itself, it's a temporary buffer owned by the caller. It also makes it clearer that what you are getting back is likely to be a managed allocation. If you'd prefer to leave string_stream_get_string() as it was, or make it return an unmanged buffer, I can send a new version. > The other option is to have a version which doesn't manage the string > at all, so just takes a gfp and requires the caller to free it (or I didn't add a companion function to get a raw unmanaged string buffer because there's nothing that needs it. It could be added later if it's needed. > register an action to do so). If we did that, we could get rid of the > struct kunit pointer totally (though it may be better to keep it and > have both versions). > Another option is to make string_stream_get_string() return a raw buffer and add a kunit_string_stream_geT_string() that returns a managed buffer. This follows some consistency with the normal mallocs where kunit_xxxx() is the managed version. > So -- to switch to some stream-of-consciousness thoughts about this -- > basically there are three possible variants of > string_stream_get_string(): > - Current version: uses the stream's struct kunit. Useless if this is > NULL, but very convenient otherwise. > - This patch: manage the string using the struct kunit passed as an > argument. Still can't be used directly outside a test, but adds enough > flexibility to get _append to work. > - Totally unmanaged: the generated string is allocated separately, and > must be freed (directly or in a deferred action) by the caller. Works > well outside tests, but less convenient. > > Personally, I feel that the design of string_stream is heading towards > the third option. By the end of this series, everything uses > _string_stream_concatenate_to_buf() anyway. There's only one call to > string_stream_get_string() outside of the logging / string_stream > tests, and all that does is log the results (and it has a fallback to > log each fragment separately if the allocation fails). > > Then again, if this is only really being used in tests, then we can > probably just stick with string_stream_get_string() as-is, remove the > string_stream->test member totally, and if we need it, we can make > _string_stream_concatenate_to_buf() public, or add an unmanaged > version anyway. > I didn't remove ->test because there's some existing code in assert.c that uses it, and I didn't want to break that. > So, after all that, I think this is probably good as-is. _Maybe_ we > could rename string_stream_get_string() to something like > string_stream_get_managed_string(), now that it's the only function > which is "managed" as a KUnit resource, but we can equally put that > off until we need to add an unmanaged version. > > Reviewed-by: David Gow <davidgow@google.com> > > Cheers, > -- David > > >> 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 >>
On 17/08/2023 07:26, David Gow wrote: > On Tue, 15 Aug 2023 at 17:57, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> >> On 15/8/23 10:16, David Gow wrote: >>> On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald >>> <rf@opensource.cirrus.com> wrote: >>>> >>>> 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 <rf@opensource.cirrus.com> >>>> --- >>> >>> Makes sense to me. >>> >>> I think that, if we weren't going to remove the struct kunit >>> dependency, we'd want to either keep a version of >>> string_stream_get_string() which uses it, or, e.g., fall back to it if >>> the struct passed in is NULL. >>> >> >> That was my first thought. But I thought that was open to subtle >> accidental bugs in calling code - it might return you a managed >> allocation, or it might return you an unmanaged allocation that you >> must free. >> >> As there weren't many callers of string_stream_get_string() I decided >> to go with changing the API to pass in test and gfp like other managed >> allocations. This makes it more generalized, since the returned buffer >> is not part of the stream itself, it's a temporary buffer owned by the >> caller. It also makes it clearer that what you are getting back is >> likely to be a managed allocation. >> >> If you'd prefer to leave string_stream_get_string() as it was, or make >> it return an unmanged buffer, I can send a new version. >> >>> The other option is to have a version which doesn't manage the string >>> at all, so just takes a gfp and requires the caller to free it (or >> >> I didn't add a companion function to get a raw unmanaged string buffer >> because there's nothing that needs it. It could be added later if >> it's needed. >> > > Fair enough. > >>> register an action to do so). If we did that, we could get rid of the >>> struct kunit pointer totally (though it may be better to keep it and >>> have both versions). >>> >> >> Another option is to make string_stream_get_string() return a raw >> buffer and add a kunit_string_stream_geT_string() that returns a >> managed buffer. This follows some consistency with the normal mallocs >> where kunit_xxxx() is the managed version. >> > > Ooh... I like this best. Let's go with that. > I was busy last week with other things and have only got back to looking at this. I'm trying to decide what to do with string_stream_get_string() and I'd value an opinion. The only use for a test-managed get_string() is the string_stream test. So I've thought of 4 options: 1) Add a kunit_string_stream_get_string() anyway. But only the test code uses it. 2) Change the tests to have a local function to do the same thing, and add an explicit test case for the (unmanaged) string_stream_get_string() that ensures it's freed. 3) Change the tests to have a local function to get the string, which calls string_stream_get_string() then copies it to a test-managed buffer and frees the unmanaged buffer. 4) Implement a kunit_kfree_on_exit() function that takes an already- allocated buffer and kfree()s it when the test exists, so that we can do: // on success kunit_kfree_on_exit() returns the buffer it was given str = kunit_kfree_on_exit(test, string_stream_get_string(stream)); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, str); I'm leaning towards (2) but open to going with any of the other options.
On Tue, 22 Aug 2023 at 00:10, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > > On 17/08/2023 07:26, David Gow wrote: > > On Tue, 15 Aug 2023 at 17:57, Richard Fitzgerald > > <rf@opensource.cirrus.com> wrote: > >> > >> On 15/8/23 10:16, David Gow wrote: > >>> On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald > >>> <rf@opensource.cirrus.com> wrote: > >>>> > >>>> 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 <rf@opensource.cirrus.com> > >>>> --- > >>> > >>> Makes sense to me. > >>> > >>> I think that, if we weren't going to remove the struct kunit > >>> dependency, we'd want to either keep a version of > >>> string_stream_get_string() which uses it, or, e.g., fall back to it if > >>> the struct passed in is NULL. > >>> > >> > >> That was my first thought. But I thought that was open to subtle > >> accidental bugs in calling code - it might return you a managed > >> allocation, or it might return you an unmanaged allocation that you > >> must free. > >> > >> As there weren't many callers of string_stream_get_string() I decided > >> to go with changing the API to pass in test and gfp like other managed > >> allocations. This makes it more generalized, since the returned buffer > >> is not part of the stream itself, it's a temporary buffer owned by the > >> caller. It also makes it clearer that what you are getting back is > >> likely to be a managed allocation. > >> > >> If you'd prefer to leave string_stream_get_string() as it was, or make > >> it return an unmanged buffer, I can send a new version. > >> > >>> The other option is to have a version which doesn't manage the string > >>> at all, so just takes a gfp and requires the caller to free it (or > >> > >> I didn't add a companion function to get a raw unmanaged string buffer > >> because there's nothing that needs it. It could be added later if > >> it's needed. > >> > > > > Fair enough. > > > >>> register an action to do so). If we did that, we could get rid of the > >>> struct kunit pointer totally (though it may be better to keep it and > >>> have both versions). > >>> > >> > >> Another option is to make string_stream_get_string() return a raw > >> buffer and add a kunit_string_stream_geT_string() that returns a > >> managed buffer. This follows some consistency with the normal mallocs > >> where kunit_xxxx() is the managed version. > >> > > > > Ooh... I like this best. Let's go with that. > > > > I was busy last week with other things and have only got back to looking > at this. > > I'm trying to decide what to do with string_stream_get_string() and I'd > value an opinion. > > The only use for a test-managed get_string() is the string_stream test. > So I've thought of 4 options: > > 1) Add a kunit_string_stream_get_string() anyway. But only the test code > uses it. > > 2) Change the tests to have a local function to do the same thing, and > add an explicit test case for the (unmanaged) > string_stream_get_string() that ensures it's freed. > > 3) Change the tests to have a local function to get the string, which > calls string_stream_get_string() then copies it to a test-managed buffer > and frees the unmanaged buffer. > > 4) Implement a kunit_kfree_on_exit() function that takes an already- > allocated buffer and kfree()s it when the test exists, so that we can > do: > > // on success kunit_kfree_on_exit() returns the buffer it was given > str = kunit_kfree_on_exit(test, string_stream_get_string(stream)); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, str); > > I'm leaning towards (2) but open to going with any of the other options. I'd be happy with any of those options (though 3 is my least favourite). There is already a kfree_at_end() function in the executor test. It's not quite as nice as your proposed kunit_kfree_on_exit() function (I like this idea of passing the pointer through), but it proves the concept. I think your version of this would be a useful thing to have as an actual part of the KUnit API, rather than just a per-test hack: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/kunit/executor_test.c#n131 Cheers, -- David
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");
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 <rf@opensource.cirrus.com> --- 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(-)