Re: [Qemu-devel] [PATCH RFC 04/10] pcnet: pcnet_common_init() always returns 0, change to void

[Gonglei]

On 2014/10/28 17:41, Markus Armbruster wrote:

> Gonglei <address@hidden> writes:
> 
>> On 2014/10/28 15:35, Markus Armbruster wrote:
>>
>>> The next commit will exploit the fact it never fails.  This one makes
>>> it obvious.
>>>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>>>  hw/net/lance.c     | 3 ++-
>>>  hw/net/pcnet-pci.c | 3 ++-
>>>  hw/net/pcnet.c     | 4 +---
>>>  hw/net/pcnet.h     | 2 +-
>>>  4 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/net/lance.c b/hw/net/lance.c
>>> index a1c49f1..3663340 100644
>>> --- a/hw/net/lance.c
>>> +++ b/hw/net/lance.c
>>> @@ -134,7 +134,8 @@ static int lance_init(SysBusDevice *sbd)
>>>  
>>>      s->phys_mem_read = ledma_memory_read;
>>>      s->phys_mem_write = ledma_memory_write;
>>> -    return pcnet_common_init(dev, s, &net_lance_info);
>>> +    pcnet_common_init(dev, s, &net_lance_info);
>>> +    return 0;
>>>  }
>>>  
>>>  static void lance_reset(DeviceState *dev)
>>> diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
>>> index fb5f5d6..50eb069 100644
>>> --- a/hw/net/pcnet-pci.c
>>> +++ b/hw/net/pcnet-pci.c
>>> @@ -335,7 +335,8 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
>>>      s->phys_mem_write = pci_physical_memory_write;
>>>      s->dma_opaque = pci_dev;
>>>  
>>> -    return pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>>> +    pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>>> +    return 0;
>>>  }
>>>  
>>>  static void pci_reset(DeviceState *dev)
>>> diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
>>> index d344c15..5a081c4 100644
>>> --- a/hw/net/pcnet.c
>>> +++ b/hw/net/pcnet.c
>>> @@ -1724,7 +1724,7 @@ void pcnet_common_cleanup(PCNetState *d)
>>>      d->nic = NULL;
>>>  }
>>>  
>>> -int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
>>> +void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo 
>>> *info)
>>
>> Do we need consider to pass an Error **errp argument to it?
> 
> This function can't fail.  The point of thimy patch is to make "can't
> fail" obvious.  If we add an errp parameter, the caller needs to check
> it, for robustness. 

Yes, it is.

> I prefer to keep things simple, and add the error
> checking only when it's actually needed.
> 

OK, it's fine :)

Reviewed-by: Gonglei <address@hidden>

Best regards,
-Gonglei