linux-kernel - Re: [PATCH] ufs: remove an unneed NULL check on list iterator

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 for Android: free password hash cracker in your pocket

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

Date:   Sun, 20 Mar 2022 01:57:39 +0100
From:   Jakob Koschel <jakobkoschel@...il.com>
To:     Xiaomeng Tong <xiam0nd.tong@...il.com>
Cc:     jejb@...ux.ibm.com, martin.petersen@...cle.com,
        Alim Akhtar <alim.akhtar@...sung.com>, avri.altman@....com,
        daejun7.park@...sung.com, bvanassche@....org, beanhuo@...ron.com,
        keosung.park@...sung.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ufs: remove an unneed NULL check on list iterator


> On 19. Mar 2022, at 05:26, Xiaomeng Tong <xiam0nd.tong@...il.com> wrote:
> 
> The list iterator is always non-NULL so it doesn't need to be checked.
> Thus just remove the unnecessary NULL check.
> 
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@...il.com>
> ---
> drivers/scsi/ufs/ufshpb.c | 6 ------
> 1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> index 2d36a0715..a3db0ad60 100644
> --- a/drivers/scsi/ufs/ufshpb.c
> +++ b/drivers/scsi/ufs/ufshpb.c
> @@ -869,12 +869,6 @@ static struct ufshpb_region *ufshpb_victim_lru_info(struct ufshpb_lu *hpb)
> 	struct ufshpb_region *rgn, *victim_rgn = NULL;
> 
> 	list_for_each_entry(rgn, &lru_info->lh_lru_rgn, list_lru_rgn) {
> -		if (!rgn) {
> -			dev_err(&hpb->sdev_ufs_lu->sdev_dev,
> -				"%s: no region allocated\n",
> -				__func__);
> -			return NULL;
> -		}

Maybe it make more sense to move that check after the loop body instead
of removing it completely? This code obviously does nothing in it's current
state but it's clear what the original intention was, so restoring that would
perhaps be better.

But probably someone with more experience about this code can make the call

> 		if (ufshpb_check_srgns_issue_state(hpb, rgn))
> 			continue;
> 
> -- 
> 2.17.1
> 

	Jakob