Re: [PATCH v2 3/3] nvme-fabrics: prevent overriding of existing host

[Max Gurtovoy <mgurtovoy@nvidia.com>]

On 12/05/2023 17:56, Christoph Hellwig wrote:
>> +
>>   	list_for_each_entry(host, &nvmf_hosts, list) {
>> +		if (!strcmp(host->nqn, hostnqn)) {
>> +			if (uuid_equal(&host->id, id)) {
>> +				// same hostnqn and hostid
>> +				return host;
>> +			} else {
>> +				pr_err("found same hostnqn %s but different hostid %pUb\n",
>> +				       hostnqn, id);
>> +				return ERR_PTR(-EINVAL);
>> +			}
>> +		} else if (uuid_equal(&host->id, id)) {
>> +			// same hostid but different hostnqn
>> +			pr_err("found same hostid %pUb but different hostnqn %s\n",
>> +			        id, hostnqn);
>> +			return ERR_PTR(-EINVAL);
>> +		}
>>   	}
> 
> Please avoid the c++ style comments.  But If the code was structured
> a little different, they might not even be beeded, i.e.
> 
> 		bool same_hostnqn = !strcmp(host->nqn, hostnqn);
> 		bool same_hostid = uuid_equal(&host->id, id);
> 
> 		if (same_hostnqn && same_hostid)
> 			return host;
> 
> 		if (same_hostnqn) {
> 			pr_err("found same hostnqn %s but different hostid %pUb\n",
> 			       hostnqn, id);
> 			return ERR_PTR(-EINVAL);
> 		}
> 		if (same_hostid) {
> 			pr_err("found same hostid %pUb but different hostnqn %s\n",
> 		        	id, hostnqn);
> 			return ERR_PTR(-EINVAL);
> 		}

Sure, good idea.
I'll address it in v3.