lkml.org 
[lkml]   [2022]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Subject[PATCH v2] nvmet: fix mar and mor off-by-one errors
Date
>>>>     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

\
 
 \ /
  Last update: 2022-09-07 09:05    [W:0.363 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site