Re: [PATCH] pci_dma_rw: return correct value instead of 0

[Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>]

On 30/07/2020 09:41, Stefano Garzarella wrote:
> On Thu, Jul 30, 2020 at 12:17:32AM +0200, Emanuele Giuseppe Esposito wrote:
>> pci_dma_rw currently always returns 0, regardless
>> of the result of dma_memory_rw. Adjusted to return
>> the correct value.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
>> ---
>>   include/hw/pci/pci.h | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> index c1bf7d5356..41c4ab5932 100644
>> --- a/include/hw/pci/pci.h
>> +++ b/include/hw/pci/pci.h
>> @@ -787,8 +787,7 @@ static inline AddressSpace *pci_get_address_space(PCIDevice *dev)
>>   static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
>>                                void *buf, dma_addr_t len, DMADirection dir)
>>   {
>> -    dma_memory_rw(pci_get_address_space(dev), addr, buf, len, dir);
>> -    return 0;
>> +    return dma_memory_rw(pci_get_address_space(dev), addr, buf, len, dir);
>>   }
> 
> I think it's a left over from when we used "void cpu_physical_memory_rw()".
> 
> I agree that it is better to return the dma_memory_rw() return value, but
> at first look, no one seems to check the return value of pci_dma_rw(),
> pci_dma_read(), andpci_dma_write().
> 
> Should we make them void?

I noticed that nobody checks the return of those functions, but I think 
checking for possible error is always useful. I am using the edu device 
and clearly doing something wrong since with this fix I discovered that 
the pci_dma_read call returns nonzero.

Keeping the function as it is or void would make it harder to spot such 
errors in future.

Thank you,
Emanuele

> 
> 
> Anyway, for this patch:
> 
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> 
> Thanks,
> Stefano
>