| Message ID | 20220319042657.21835-1-xiam0nd.tong@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ufs: remove an unneed NULL check on list iterator | expand |
> On 19. Mar 2022, at 05:26, Xiaomeng Tong <xiam0nd.tong@gmail.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@gmail.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
On Sun, 20 Mar 2022 01:57:39 +0100, Jakob Koschel <jakobkoschel@gmail.com> wrote: > 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 > This seems like a better choice, if it doesn't go against the author's original intention. I will fix it in PATCH v2, thanks. -- Xiaomeng Tong
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; - } if (ufshpb_check_srgns_issue_state(hpb, rgn)) continue;
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@gmail.com> --- drivers/scsi/ufs/ufshpb.c | 6 ------ 1 file changed, 6 deletions(-)