From: David Sterba <dsterba@suse.cz>
To: Filipe Manana <fdmanana@kernel.org>
Cc: David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 08/10] btrfs: simplify conditions in btrfs_free_chunk_map()
Date: Mon, 19 Feb 2024 15:41:21 +0100 [thread overview]
Message-ID: <20240219144121.GV355@twin.jikos.cz> (raw)
In-Reply-To: <CAL3q7H6MyuNBkRaYHzwid6ccOSYC0Yym2VgwozsqZbVS_6Fzvw@mail.gmail.com>
On Mon, Feb 19, 2024 at 12:27:44PM +0000, Filipe Manana wrote:
> On Mon, Feb 19, 2024 at 11:14 AM David Sterba <dsterba@suse.com> wrote:
> >
> > The helper is simple enough for inlining, we can further simplify it by
> > removing the check for map pointer validity. After this patch all
> > callers always pass a valid pointer.
>
> So by making btrfs_free_chunk_map() to not ignore a NULL pointer, we
> are adding rather
> surprising behaviour and inconsistency.
>
> Most free functions ignore a NULL pointer, take the example of the
> kfree() family and even free() family in the standard C library,
> as well as most of the free functions we have in btrfs as well, which
> are modeled on that common pattern.
>
> Ignoring NULL makes error handling simpler, by not having the need to
> take special care to call the free function with a non-NULL pointer.
>
> Besides that, this change doesn't seem to improve anything.
The goal is to reduce a static inline function size as its code is
duplicated many times (in this case 36x) so anything that does not need
to be there is removed. The improvement is smaller code size, one less
condition to check.
OTOH that it's a freeing function and would not accept a NULL pointer is
indeed inconsistent and potentially problematic so I'll drop the patch.
next prev parent reply other threads:[~2024-02-19 14:41 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 11:12 [PATCH 00/10] Static inline cleanups David Sterba
2024-02-19 11:12 ` [PATCH 01/10] btrfs: move balance args conversion helpers to volumes.c David Sterba
2024-02-19 11:12 ` [PATCH 02/10] btrfs: open code btrfs_backref_iter_free() David Sterba
2024-02-19 11:12 ` [PATCH 03/10] btrfs: open code btrfs_backref_get_eb() David Sterba
2024-02-19 11:12 ` [PATCH 04/10] btrfs: uninline some static inline helpers from backref.h David Sterba
2024-02-19 11:12 ` [PATCH 05/10] btrfs: uninline btrfs_init_delayed_root() David Sterba
2024-02-19 11:12 ` [PATCH 06/10] btrfs: drop static inline specifiers from tree-mod-log.c David Sterba
2024-02-19 11:13 ` [PATCH 07/10] btrfs: uninline some static inline helpers from tree-log.h David Sterba
2024-02-19 11:13 ` [PATCH 08/10] btrfs: simplify conditions in btrfs_free_chunk_map() David Sterba
2024-02-19 12:27 ` Filipe Manana
2024-02-19 14:41 ` David Sterba [this message]
2024-02-26 8:31 ` kernel test robot
2024-02-19 11:13 ` [PATCH 09/10] btrfs: open code trivial btrfs_lru_cache_size() David Sterba
2024-02-19 11:13 ` [PATCH 10/10] btrfs: uninline some static inline helpers from delayed-ref.h 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=20240219144121.GV355@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=dsterba@suse.com \
--cc=fdmanana@kernel.org \
--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.