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
>
