Thanks for reviewing this patch Fred,
Frederic Barrat <fbar...@linux.ibm.com> writes:
>>
>> afu_result = cxl_vphb_error_detected(afu, state);
>>
>> - cxl_context_detach_all(afu);
>> - cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>> - pci_deconfigure_afu(afu);
>> + if (afu != NULL) {
>> + cxl_context_detach_all(afu);
>> + cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>> + pci_deconfigure_afu(afu);
>> + }
>
> I can see you're also checking if cxl_vphb_error_detected() is called
> with a NULL afu from within the function, but why not move the call to
> cxl_vphb_error_detected() within that "if (afu != NULL)... " statement?
> Otherwise, it looks suspicious when reading the code.
Yes, agree. However this was triggerring gcc compile warning
'maybe-uninitialized' for 'afu_result', hence removed the call to
cxl_vphb_error_detected() outside the branch. Have fixed this in v2 with
an explicit initialization of 'afu_result'.
>
>
>> @@ -2051,10 +2067,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>> * This is not the place to be checking if everything came back up
>> * properly, because there's no return value: do that in slot_reset.
>> */
>> + spin_lock(&adapter->afu_list_lock);
>> for (i = 0; i < adapter->slices; i++) {
>> afu = adapter->afu[i];
>>
>> - if (afu->phb == NULL)
>> + if (afu || afu->phb == NULL)
>> continue;
>
>
> if (afu == NULL ...
Thanks for catching this. Have fixed this in v2.
--
Vaibhav Jain <vaib...@linux.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.
