[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pci_dma_rw: return correct value instead of 0
From: |
Stefano Garzarella |
Subject: |
Re: [PATCH] pci_dma_rw: return correct value instead of 0 |
Date: |
Thu, 30 Jul 2020 12:41:10 +0200 |
On Thu, Jul 30, 2020 at 10:50:43AM +0200, Emanuele Giuseppe Esposito wrote:
>
>
> 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.
I agree, I was just worried that no one checks the return value.
Thanks,
Stefano