Messages in this thread | | | From | Dennis Maisenbacher <> | Subject | [PATCH v2] nvmet: fix mar and mor off-by-one errors | Date | Wed, 7 Sep 2022 07:04:12 +0000 |
| |
>>>> if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) { >>>> req->error_loc = offsetof(struct nvme_identify, nsid); >>>> @@ -130,8 +131,20 @@ void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req) >>>> zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >> >>>> req->ns->blksize_shift; >>>> id_zns->lbafe[0].zsze = cpu_to_le64(zsze); >>>> - id_zns->mor = cpu_to_le32(bdev_max_open_zones(req->ns->bdev)); >>>> - id_zns->mar = cpu_to_le32(bdev_max_active_zones(req->ns->bdev)); >>>> + >>>> + mor = bdev_max_open_zones(req->ns->bdev); >>>> + if (!mor) >>>> + mor = U32_MAX; >>>> + else >>>> + --mor; >>>> + id_zns->mor = cpu_to_le32(mor); >>>> + >>>> + mar = bdev_max_active_zones(req->ns->bdev); >>>> + if (!mar) >>>> + mar = U32_MAX; >>>> + else >>>> + --mar; >>>> + id_zns->mar = cpu_to_le32(mar); >>>> >>> >>> above 14 lines of code can be simplified as in 4-5 lines :- >> >> Simplified ? It is much harder to read in my opinion... >> >>> > >there are two if ... else ... doing identical things on same data >type u32 and its return type is also same le32, >if my suggestion is hard to read then common code needs >to be moved to the helper as it is not clear the need for >code duplication from commit message.
For my taste the conditional operator would have looked like a good tradeoff in this case. It condenses the code and verbosely converts a 0 to U32_MAX. A new helper would have read like the conditional operator.
Christoph already applied the patch. In any case thanks for your comments!
Dennis
| |