linux-kernel - Re: [PATCH v3] PCI: vmd: Assign a number to each VMD controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b87f7a38-d147-3da6-0352-ad2df7a5f55f@gmail.com>
Date:   Fri, 17 Sep 2021 09:36:08 +0800
From:   brookxu <brookxu.cn@...il.com>
To:     Krzysztof Wilczyński <kw@...ux.com>
Cc:     jonathan.derrick@...el.com, lorenzo.pieralisi@....com,
        robh@...nel.org, bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] PCI: vmd: Assign a number to each VMD controller



Krzysztof Wilczyński wrote on 2021/9/17 6:57 上午:
> Hi Xu,
> 
> Thank you for sending the patch over!
> 
> A small nitpick below, so feel free to ignore it.
> 
> [...] 
>> @@ -769,28 +773,48 @@ static int vmd_probe(struct pci_dev *dev, const struct pci_device_id *id)
>>  {
>>  	unsigned long features = (unsigned long) id->driver_data;
>>  	struct vmd_dev *vmd;
>> -	int err;
>> +	int err = 0;
>>  
>> -	if (resource_size(&dev->resource[VMD_CFGBAR]) < (1 << 20))
>> -		return -ENOMEM;
>> +	if (resource_size(&dev->resource[VMD_CFGBAR]) < (1 << 20)) {
>> +		err = -ENOMEM;
>> +		goto out;
>> +	}
>>  
>>  	vmd = devm_kzalloc(&dev->dev, sizeof(*vmd), GFP_KERNEL);
>> -	if (!vmd)
>> -		return -ENOMEM;
>> +	if (!vmd) {
>> +		err = -ENOMEM;
>> +		goto out;
>> +	}
> 
> I assume that you changed the above to use the newly added "out" label to
> be consistent given that you also have the other label, but since there is
> no clean-up to be done here, do we need this additional label?
> 
>>  	vmd->dev = dev;
>> +	vmd->instance = ida_simple_get(&vmd_instance_ida, 0, 0, GFP_KERNEL);
>> +	if (vmd->instance < 0) {
>> +		err = vmd->instance;
>> +		goto out;
>> +	}
> 
> Similarly to here to the above, no clean-up to be done, and you could just
> return immediately here.
> 
> What do you think?
> 

Thanks, I think we can do this.

> Also, I think we might have lost a "Reviewed-by" from Jon Derrick somewhere
> along the way.  Given that you only updated the commit log and the subject
> like, it probably still applies (unless Jon would like to give his seal of
> approval again).
> 

Thanks, my mistake here.

> 	Krzysztof
>