linux-kernel - btree: fix merge logic to use btree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250826160826.686111-1-409411716@gms.tku.edu.tw>
Date: Wed, 27 Aug 2025 00:08:26 +0800
From: Guan-Chun Wu <409411716@....tku.edu.tw>
To: visitorckw@...il.com
Cc: 409411716@....tku.edu.tw,
	akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: btree: fix merge logic to use btree_last() return value

Hi Kuan-Wei,

Thanks for the review.

> On Fri, Aug 22, 2025 at 04:58:51PM +0800, Guan-Chun.Wu wrote:
>> Previously btree_merge() called btree_last() only to test existence,
>> then performed an extra btree_lookup() to fetch the value. This patch
>> changes it to directly use the value returned by btree_last(), avoiding
>> redundant lookups and simplifying the merge loop.
>
> The code change itself looks correct.
>
> However, the subject line gives the impression that this patch fixes a
> bug, while it actually just simplifies the logic. Could you consider
> updating the subject to better reflect the nature of the change?

Good point. I will update the subject to:
    btree: simplify merge logic by using btree_last() return value

> BTW, it seems that only the qla2xxx SCSI driver uses the btree
> library, and that driver doesn't actually call btree_merge(). So in
> practice, this function is unused in the kernel. Should we consider
> removing it entirely?

That makes sense. Since btree_merge() is currently unused, maybe it's
worth considering removal in a follow-up patch.

> Signed-off-by: Guan-Chun.Wu <409411716@....tku.edu.tw>
>
> Is the dot in your real name intentional?

No, it was unintentional. My correct name should be "Guan-Chun Wu"
(without the dot). I have updated my git config so future submissions
will use the correct name.

Thanks,
Guan-Chun

> ---
>  lib/btree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/btree.c b/lib/btree.c
> index bb81d3393ac5..9c80c0c7bba8 100644
> --- a/lib/btree.c
> +++ b/lib/btree.c
> @@ -653,9 +653,9 @@ int btree_merge(struct btree_head *target, struct btree_head *victim,
>        * walks to remove a single object from the victim.
>        */
>       for (;;) {
> -         if (!btree_last(victim, geo, key))
> +         val = btree_last(victim, geo, key);
> +         if (!val)
>               break;
> -         val = btree_lookup(victim, geo, key);
>           err = btree_insert(target, geo, key, val, gfp);
>           if (err)
>               return err;
> -- 
> 2.34.1