[PATCH 9/9] nvme-core: fix nvme_submit_sync_cmd() args

Chaitanya Kulkarni chaitanyak at nvidia.com
Thu Mar 23 01:23:34 PDT 2023


On 3/23/23 01:16, Sagi Grimberg wrote:
>> The __nvme_submit_sync_cmd() function currently has a long list of
>> eight arguments, which can make it difficult to read and debug,
>> particularly when the function is commonly used from different parts
>> of the codebase. This complexity can also increase when extending the
>> functionality of the function, resulting in more arguments being added.
>
> I don't think that complexity/difficulty is the issue here, its just
> ugly.
>
>>
>> To address this issue, we can create a new structure called
>> nvme_submit_cmd_data
>
> Maybe call it nvme_sync_cmd_args.

okay.

>
>> and move all the existing arguments into this
>> structure. By doing this, we can pass the structure as a single argument
>> to the __nvme_submit_sync_cmd() function. This approach simplifies the
>> code and makes it more readable, and also provides a way to add future
>> arguments to the structure without increasing the number of function
>> arguments.
>
> The potential gain here is that on-stack initialized structs you can
> implcitly zero-out members so you don't need to explicitly set in all
> the call sites. But that is a subjective gain because it can be counter
> intuitive at times. Otherwise there is no real coding gain with this
> specifically.
>
> But regardless, this looks fine to me.

exactly, first version I created was with only required parameters
initialized, I found this one much easy to read.

-ck




More information about the Linux-nvme mailing list