From: Leo Martins <loemra.dev@gmail.com>
To: kernel-team@fb.com, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v3 0/3] btrfs path auto free
Date: Fri, 30 Aug 2024 13:46:59 -0700 [thread overview]
Message-ID: <j1u38.er61zc06h8sh@gmail.com> (raw)
In-Reply-To: <cover.1724792563.git.loemra.dev@gmail.com>
I think the only feedback I haven't addressed in this patchset was
moving the declaration to be next to the btrfs_path struct.
However, I don't think moving the declaration makes sense because
the DEFINE_FREE references btrfs_free_path which itself is only
declared after the structure. The other examples I've looked at also
have DEFINE_FREE next to the freeing function as opposed to the
structure being freed.
On Tue, 27 Aug 2024 15:41, Leo Martins <loemra.dev@gmail.com> wrote:
>The DEFINE_FREE macro defines a wrapper function for a given memory
>cleanup function which takes a pointer as an argument and calls the
>cleanup function with the value of the pointer. The __free macro adds
>a scoped-based cleanup to a variable, using the __cleanup attribute
>to specify the cleanup function that should be called when the variable
>goes out of scope.
>
>Using this cleanup code pattern ensures that memory is properly freed
>when it's no longer needed, preventing memory leaks and reducing the
>risk of crashes or other issues caused by incorrect memory management.
>Even if the code is already memory safe, using this pattern reduces
>the risk of introducing memory-related bugs in the future
>
>In this series of patches I've added a DEFINE_FREE for btrfs_free_path
>and created a macro BTRFS_PATH_AUTO_FREE to clearly identify path
>declarations that will be automatically freed.
>
>I've included some simple examples of where this pattern can be used.
>The trivial examples are ones where there is one exit path and the only
>cleanup performed is a call to btrfs_free_path.
>
>There appear to be around 130 instances that would be a pretty simple to
>convert to this pattern. Is it worth going back and updating
>all trivial instances or would it be better to leave them and use the pattern
>in new code? Another option is to have all path declarations declared
>with BTRFS_PATH_AUTO_FREE and not remove any btrfs_free_path instances.
>In theory this would not change the functionality as it is fine to call
>btrfs_free_path on an already freed path.
>
>Leo Martins (3):
> btrfs: DEFINE_FREE for btrfs_free_path
> btrfs: BTRFS_PATH_AUTO_FREE in zoned.c
> btrfs: BTRFS_PATH_AUTO_FREE in orphan.c
>
> fs/btrfs/ctree.c | 2 +-
> fs/btrfs/ctree.h | 4 ++++
> fs/btrfs/orphan.c | 19 ++++++-------------
> fs/btrfs/zoned.c | 34 +++++++++++-----------------------
> 4 files changed, 22 insertions(+), 37 deletions(-)
>
>--
>2.43.5
>
next prev parent reply other threads:[~2024-08-30 20:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-27 22:41 [PATCH v3 0/3] btrfs path auto free Leo Martins
2024-08-27 22:41 ` [PATCH v3 1/3] btrfs: DEFINE_FREE for btrfs_free_path Leo Martins
2024-08-27 22:41 ` [PATCH v3 2/3] btrfs: BTRFS_PATH_AUTO_FREE in zoned.c Leo Martins
2024-08-27 22:41 ` [PATCH v3 3/3] btrfs: BTRFS_PATH_AUTO_FREE in orphan.c Leo Martins
2024-08-28 16:02 ` [PATCH v3 0/3] btrfs path auto free David Sterba
2024-08-30 20:46 ` Leo Martins [this message]
2024-09-02 23:43 ` David Sterba
2024-09-02 23:59 ` David Sterba
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=j1u38.er61zc06h8sh@gmail.com \
--to=loemra.dev@gmail.com \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.