public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: David Gow <davidgow@google.com>
Cc: <brendan.higgins@linux.dev>, <rmoar@google.com>,
	<linux-kselftest@vger.kernel.org>, <kunit-dev@googlegroups.com>,
	<linux-kernel@vger.kernel.org>, <patches@opensource.cirrus.com>
Subject: Re: [PATCH v5 08/10] kunit: string-stream: Add tests for freeing resource-managed string_stream
Date: Fri, 25 Aug 2023 10:41:38 +0100	[thread overview]
Message-ID: <3ce4262e-729c-932e-cee7-db07305550d1@opensource.cirrus.com> (raw)
In-Reply-To: <CABVgOSnG8epucY52ppNW7-vC_-QL3Lau9zsaw20dXgWTC0GV+Q@mail.gmail.com>

On 25/08/2023 07:49, David Gow wrote:
> On Thu, 24 Aug 2023 at 22:32, 'Richard Fitzgerald' via KUnit
> Development <kunit-dev@googlegroups.com> wrote:
>>
>> string_stream_managed_free_test() allocates a resource-managed
>> string_stream and tests that kunit_free_string_stream() calls
>> string_stream_destroy().
>>
>> string_stream_resource_free_test() allocates a resource-managed
>> string_stream and tests that string_stream_destroy() is called
>> when the test resources are cleaned up.
>>
>> The old string_stream_init_test() has been split into two tests,
>> one for kunit_alloc_string_stream() and the other for
>> alloc_string_stream().
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> ---
>> Changes since V4:
>> - Added test case for kunit_free_string_stream().
>> - Split the initialization test into separate tests for managed and
>>    unmanaged allocations.
>> ---
> 
> Looking over this again, I'm not convinced the streams are actually
> getting freed. Once the stub has finished, the stream is removed from
> the list of deferred actions / resources.
> 

Argh, I think you're right. My original version stashed the stream into
the private data and freed it in a test exit() function so that it was
guaranteed to be freed even if the resource cleanup wasn't called and
the test function aborted before it could do a manual cleanup.
I decided to simplify that but actually that original implementation was
better.


  reply	other threads:[~2023-08-25  9:42 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-24 14:31 [PATCH v5 00/10] kunit: Add dynamically-extending log Richard Fitzgerald
2023-08-24 14:31 ` [PATCH v5 01/10] kunit: string-stream: Don't create a fragment for empty strings Richard Fitzgerald
2023-08-24 22:42   ` Rae Moar
2023-08-25  6:49   ` David Gow
2023-08-24 14:31 ` [PATCH v5 02/10] kunit: string-stream: Improve testing of string_stream Richard Fitzgerald
2023-08-24 22:42   ` Rae Moar
2023-08-25 14:58     ` Richard Fitzgerald
2023-08-25  6:49   ` David Gow
2023-08-25  9:32     ` Richard Fitzgerald
2023-08-24 14:31 ` [PATCH v5 03/10] kunit: string-stream: Add option to make all lines end with newline Richard Fitzgerald
2023-08-24 23:13   ` Rae Moar
2023-08-25  6:49   ` David Gow
2023-08-24 14:31 ` [PATCH v5 04/10] kunit: string-stream: Add cases for string_stream newline appending Richard Fitzgerald
2023-08-24 23:22   ` Rae Moar
2023-08-25  6:49   ` David Gow
2023-08-24 14:31 ` [PATCH v5 05/10] kunit: Don't use a managed alloc in is_literal() Richard Fitzgerald
2023-08-25  6:49   ` David Gow
2023-08-24 14:31 ` [PATCH v5 06/10] kunit: string-stream: Add kunit_alloc_string_stream() Richard Fitzgerald
2023-08-25  6:49   ` David Gow
2023-08-24 14:31 ` [PATCH v5 07/10] kunit: string-stream: Decouple string_stream from kunit Richard Fitzgerald
2023-08-25  6:17   ` kernel test robot
2023-08-25  6:49   ` David Gow
2023-08-24 14:31 ` [PATCH v5 08/10] kunit: string-stream: Add tests for freeing resource-managed string_stream Richard Fitzgerald
2023-08-25  6:49   ` David Gow
2023-08-25  9:41     ` Richard Fitzgerald [this message]
2023-08-24 14:31 ` [PATCH v5 09/10] kunit: Use string_stream for test log Richard Fitzgerald
2023-08-25  6:17   ` kernel test robot
2023-08-25  6:53   ` David Gow
2023-08-25  7:30   ` kernel test robot
2023-08-24 14:31 ` [PATCH v5 10/10] kunit: string-stream: Test performance of string_stream Richard Fitzgerald
2023-08-25  6:50   ` David Gow
2023-08-25  6:58 ` [PATCH v5 00/10] kunit: Add dynamically-extending log David Gow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ce4262e-729c-932e-cee7-db07305550d1@opensource.cirrus.com \
    --to=rf@opensource.cirrus.com \
    --cc=brendan.higgins@linux.dev \
    --cc=davidgow@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=rmoar@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox