Re: [RFC PATCH 1/2] nvme-core: factor out common code into helper

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

On 5/1/23 23:11, Hannes Reinecke wrote:
> On 4/27/23 10:02, Chaitanya Kulkarni wrote:
>> nvme_ctrl_dhchap_secrete_store() & nvme_ctrl_dhchap_ctrl_secret store()
>> share common code. Instead of repeating code into two functions factor
>> out into helper function.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>> ---
>>   drivers/nvme/host/core.c | 85 +++++++++++++++-------------------------
>>   1 file changed, 31 insertions(+), 54 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 44408c0c1762..171ae1f2197a 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -3779,6 +3779,7 @@ static ssize_t dctype_show(struct device *dev,
>>   static DEVICE_ATTR_RO(dctype);
>>     #ifdef CONFIG_NVME_AUTH
>> +
>>   static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
>>           struct device_attribute *attr, char *buf)
>>   {
>> @@ -3790,41 +3791,40 @@ static ssize_t 
>> nvme_ctrl_dhchap_secret_show(struct device *dev,
>>       return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
>>   }
>>   -static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
>> -        struct device_attribute *attr, const char *buf, size_t count)
>> +static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
>> +        char **dhchap_secret, struct nvme_dhchap_key **orig_key,
>> +        const char *buf, size_t count)
>
> Hmm. I'm _not_ a big fan of using double pointer in arguments; is always
> feels to me like using the wrong abstraction.
> Can't you just pass in the ctrl and use a 'bool' variable to switch 
> between host and controller secret?
> Interface is _far_ cleaner, and the resulting code will be, too, as we 
> don't have to deal with pointer variables all the time ...
>
> Cheers,
>
> Hannes


sounds good, I don't see the reason why bool will not work,
I'll sendout v1 with bool and removing double pointer..


thanks for taking a look ..

-ck