linux-kernel - Re: 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: <aKyVQRG9k/CboNRn@visitorckw-System-Product-Name>
Date: Tue, 26 Aug 2025 00:54:25 +0800
From: Kuan-Wei Chiu <visitorckw@...il.com>
To: "Guan-Chun.Wu" <409411716@....tku.edu.tw>
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: btree: fix merge logic to use btree_last() return value

Hi Guan-Chun,

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?

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?

> 
> Signed-off-by: Guan-Chun.Wu <409411716@....tku.edu.tw>

Is the dot in your real name intentional?

Regards,
Kuan-Wei

> ---
>  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
> 
>