Re: [PATCH v4 07/10] kunit: string-stream: Decouple string_stream from kunit

Richard Fitzgerald
2023-08-15T10:53:02Z

On 15/8/23 10:16, David Gow wrote:
> On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald
> <rf@opensource.cirrus.com> wrote:
>>
>> 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 <rf@opensource.cirrus.com>
>> ---
>
> I'm in favour of this. Should we go further and get rid of the struct
> kunit member from string_stream totally?
>

I can do that. I was worried about some hairy-looking code in assert.c
that used stream->test. But I've just looked at it again and it's
really quite simple, and doesn't even need ->test. is_literal()
allocates a temporary managed buffer, but it frees it before returning
so it doesn't need to be managed.

> Also, note that the kunit_action_t casting is causing warnings on some
> clang configs (and technically isn't valid C). Personally, I still
> like it, but expect more emails from the kernel test robot and others.
>

I can send a new version to fix that.