On Tue, 15 Aug 2023 at 18:51, Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On 15/8/23 10:16, David Gow wrote: > > On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald > > <rf@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx> > >> --- > > > > 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. > Yeah, let's get rid of that. Having a stream->kunit exist but be NULL half the time is asking for issues down the line. > > 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. > That's probably best. If you want to keep it as-is, I'll fight for it, but it's probably better to err on the side of not introducing the warnings. Thanks, -- David
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
