Changes since v1:
  - Remove the _PTR suffix
  - Rename the ipath cleanup function to inode_fs_paths, so it's more
    explicit on the type.
  - Improve git message in patch 1.

This patchset introduces and applies throughout the btrfs tree two new
macros: AUTO_KFREE and AUTO_KVFREE. Each macro defines a pointer,
initializes it to NULL, and sets the kfree/kvfree cleanup attribute. It was
suggested by David Sterba in the review of a patch that I submitted here
[1].

I have not applied these macros blindly through the tree, but only when
using a cleanup attribute actually made things easier for
maintainers/developers, and didn't obfuscate things like lifetimes of
objects on a given function. So, I've mostly avoided applying this when:

- The object was being re-allocated in the middle of the function
  (e.g. object re-allocation in a loop).
- The ownership of the object was transferred between functions.
- The value of a given object might depend on functions returning ERR_PTR()
  et al.
- The cleanup section of a function was a bunch of labels with different
  exit paths with non-trivial cleanup code (or code that depended on things
  to go on a specific order).

To come up with this patchset I have glanced through the tree in order to
find where and how kfree()/kvfree() were being used, and while doing so I
have submitted [2], [3] and [4] separately as they were fixing memory
related issues. All in all, this patchset can be divided in three parts:

1. Patch 1: transforms free_ipath() to be defined via DEFINE_FREE(), which
   will be useful in order to further simplify some code in patch 3.
2. Patch 2 and 3: define and use the two macros.
3. Patch 4: removing some unneeded kfree() calls from qgroup.c as they were
   not needed. Since these occurrences weren't memory bugs, and it was a
   somewhat simple patch, I've refrained from sending this separately as I
   did in [2], [3] and [4]; but I'll gladly do it if you think it's better
   for the review.

Note that after these changes some 'return' statements could be made more
explicit, and I've also written an explicit 'return 0' whenever it would
make more explicit the "happy" path for a given branch, or whenever a 'ret'
variable could be avoided that way.

Last, checkpatch.pl script doesn't seem to like patches 2 and 3; but so far
it looks like false positives to me. But of course I might just be wrong :)

[1] https://lore.kernel.org/all/20250922103442.GM5333@twin.jikos.cz/
[2] https://lore.kernel.org/all/20250925184139.403156-1-mssola@mssola.com/
[3] https://lore.kernel.org/all/20250930130452.297576-1-mssola@mssola.com/
[4] https://lore.kernel.org/all/20251008121859.440161-1-mssola@mssola.com/

Miquel Sabaté Solà (4):
  btrfs: declare free_ipath() via DEFINE_FREE()
  btrfs: define the AUTO_K(V)FREE helper macros
  btrfs: apply the AUTO_K(V)FREE macros throughout the tree
  btrfs: add ASSERTs on prealloc in qgroup functions

 fs/btrfs/acl.c                    | 29 ++++++++----------
 fs/btrfs/backref.c                | 10 +------
 fs/btrfs/backref.h                |  7 ++++-
 fs/btrfs/delayed-inode.c          | 17 +++++------
 fs/btrfs/extent-tree.c            | 17 +++++------
 fs/btrfs/inode.c                  |  4 +--
 fs/btrfs/ioctl.c                  | 34 ++++++++-------------
 fs/btrfs/misc.h                   |  7 +++++
 fs/btrfs/qgroup.c                 | 30 +++++++++++++++----
 fs/btrfs/raid-stripe-tree.c       | 14 +++------
 fs/btrfs/reflink.c                |  7 ++---
 fs/btrfs/relocation.c             | 34 ++++++++-------------
 fs/btrfs/scrub.c                  |  4 +--
 fs/btrfs/send.c                   | 50 ++++++++++++-------------------
 fs/btrfs/super.c                  |  3 +-
 fs/btrfs/tests/extent-io-tests.c  |  3 +-
 fs/btrfs/tests/extent-map-tests.c |  6 ++--
 fs/btrfs/tree-log.c               | 46 +++++++++++-----------------
 fs/btrfs/volumes.c                | 28 +++++------------
 fs/btrfs/zoned.c                  |  3 +-
 20 files changed, 145 insertions(+), 208 deletions(-)

--
2.51.1

On Fri, Oct 24, 2025 at 12:21:39PM +0200, Miquel Sabaté Solà wrote:
> Changes since v1:
>   - Remove the _PTR suffix
>   - Rename the ipath cleanup function to inode_fs_paths, so it's more
>     explicit on the type.
>   - Improve git message in patch 1.
> 
> This patchset introduces and applies throughout the btrfs tree two new
> macros: AUTO_KFREE and AUTO_KVFREE. Each macro defines a pointer,
> initializes it to NULL, and sets the kfree/kvfree cleanup attribute. It was
> suggested by David Sterba in the review of a patch that I submitted here
> [1].
> 
> I have not applied these macros blindly through the tree, but only when
> using a cleanup attribute actually made things easier for
> maintainers/developers, and didn't obfuscate things like lifetimes of
> objects on a given function. So, I've mostly avoided applying this when:
> 
> - The object was being re-allocated in the middle of the function
>   (e.g. object re-allocation in a loop).
> - The ownership of the object was transferred between functions.
> - The value of a given object might depend on functions returning ERR_PTR()
>   et al.
> - The cleanup section of a function was a bunch of labels with different
>   exit paths with non-trivial cleanup code (or code that depended on things
>   to go on a specific order).
> 
> To come up with this patchset I have glanced through the tree in order to
> find where and how kfree()/kvfree() were being used, and while doing so I
> have submitted [2], [3] and [4] separately as they were fixing memory
> related issues. All in all, this patchset can be divided in three parts:
> 
> 1. Patch 1: transforms free_ipath() to be defined via DEFINE_FREE(), which
>    will be useful in order to further simplify some code in patch 3.
> 2. Patch 2 and 3: define and use the two macros.
> 3. Patch 4: removing some unneeded kfree() calls from qgroup.c as they were
>    not needed. Since these occurrences weren't memory bugs, and it was a
>    somewhat simple patch, I've refrained from sending this separately as I
>    did in [2], [3] and [4]; but I'll gladly do it if you think it's better
>    for the review.
> 
> Note that after these changes some 'return' statements could be made more
> explicit, and I've also written an explicit 'return 0' whenever it would
> make more explicit the "happy" path for a given branch, or whenever a 'ret'
> variable could be avoided that way.
> 
> Last, checkpatch.pl script doesn't seem to like patches 2 and 3; but so far
> it looks like false positives to me. But of course I might just be wrong :)
> 
> [1] https://lore.kernel.org/all/20250922103442.GM5333@twin.jikos.cz/
> [2] https://lore.kernel.org/all/20250925184139.403156-1-mssola@mssola.com/
> [3] https://lore.kernel.org/all/20250930130452.297576-1-mssola@mssola.com/
> [4] https://lore.kernel.org/all/20251008121859.440161-1-mssola@mssola.com/
> 
> Miquel Sabaté Solà (4):
>   btrfs: declare free_ipath() via DEFINE_FREE()
>   btrfs: define the AUTO_K(V)FREE helper macros
>   btrfs: apply the AUTO_K(V)FREE macros throughout the tree
>   btrfs: add ASSERTs on prealloc in qgroup functions

Thanks, patches now added to for-next with some minor adjustments. Feel
free to send more conversions, there are still some kvfree candidate
calls. I think we would not mind using it even for the short functions
(re what's mentioned in the 3rd patch), so it's established as a common
coding pattern. This change has net negative effect on lines and also
simplifies the control flow.

David Sterba @ 2025-10-31 03:22 +01:

> On Fri, Oct 24, 2025 at 12:21:39PM +0200, Miquel Sabaté Solà wrote:
>> Changes since v1:
>>   - Remove the _PTR suffix
>>   - Rename the ipath cleanup function to inode_fs_paths, so it's more
>>     explicit on the type.
>>   - Improve git message in patch 1.
>>
>> This patchset introduces and applies throughout the btrfs tree two new
>> macros: AUTO_KFREE and AUTO_KVFREE. Each macro defines a pointer,
>> initializes it to NULL, and sets the kfree/kvfree cleanup attribute. It was
>> suggested by David Sterba in the review of a patch that I submitted here
>> [1].
>>
>> I have not applied these macros blindly through the tree, but only when
>> using a cleanup attribute actually made things easier for
>> maintainers/developers, and didn't obfuscate things like lifetimes of
>> objects on a given function. So, I've mostly avoided applying this when:
>>
>> - The object was being re-allocated in the middle of the function
>>   (e.g. object re-allocation in a loop).
>> - The ownership of the object was transferred between functions.
>> - The value of a given object might depend on functions returning ERR_PTR()
>>   et al.
>> - The cleanup section of a function was a bunch of labels with different
>>   exit paths with non-trivial cleanup code (or code that depended on things
>>   to go on a specific order).
>>
>> To come up with this patchset I have glanced through the tree in order to
>> find where and how kfree()/kvfree() were being used, and while doing so I
>> have submitted [2], [3] and [4] separately as they were fixing memory
>> related issues. All in all, this patchset can be divided in three parts:
>>
>> 1. Patch 1: transforms free_ipath() to be defined via DEFINE_FREE(), which
>>    will be useful in order to further simplify some code in patch 3.
>> 2. Patch 2 and 3: define and use the two macros.
>> 3. Patch 4: removing some unneeded kfree() calls from qgroup.c as they were
>>    not needed. Since these occurrences weren't memory bugs, and it was a
>>    somewhat simple patch, I've refrained from sending this separately as I
>>    did in [2], [3] and [4]; but I'll gladly do it if you think it's better
>>    for the review.
>>
>> Note that after these changes some 'return' statements could be made more
>> explicit, and I've also written an explicit 'return 0' whenever it would
>> make more explicit the "happy" path for a given branch, or whenever a 'ret'
>> variable could be avoided that way.
>>
>> Last, checkpatch.pl script doesn't seem to like patches 2 and 3; but so far
>> it looks like false positives to me. But of course I might just be wrong :)
>>
>> [1] https://lore.kernel.org/all/20250922103442.GM5333@twin.jikos.cz/
>> [2] https://lore.kernel.org/all/20250925184139.403156-1-mssola@mssola.com/
>> [3] https://lore.kernel.org/all/20250930130452.297576-1-mssola@mssola.com/
>> [4] https://lore.kernel.org/all/20251008121859.440161-1-mssola@mssola.com/
>>
>> Miquel Sabaté Solà (4):
>>   btrfs: declare free_ipath() via DEFINE_FREE()
>>   btrfs: define the AUTO_K(V)FREE helper macros
>>   btrfs: apply the AUTO_K(V)FREE macros throughout the tree
>>   btrfs: add ASSERTs on prealloc in qgroup functions
>
> Thanks, patches now added to for-next with some minor adjustments. Feel
> free to send more conversions, there are still some kvfree candidate
> calls. I think we would not mind using it even for the short functions
> (re what's mentioned in the 3rd patch), so it's established as a common
> coding pattern. This change has net negative effect on lines and also
> simplifies the control flow.

Thanks for adding them!

Will keep an eye then if there are places where it's safe to use them.

Greetings,
Miquel