Mao Zhongyi <maozy.f...@cn.fujitsu.com> writes: > Hi, Eduardo > > On 06/06/2017 10:52 PM, Eduardo Habkost wrote: >> On Tue, Jun 06, 2017 at 07:26:30PM +0800, Mao Zhongyi wrote: >>> Add Error argument for pci_add_capability() to leverage the errp >>> to pass info on errors. This way is helpful for its callers to >>> make a better error handling when moving to 'realize'. [...] >>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >>> index b73bfea..2bba37a 100644 >>> --- a/hw/pci/pci.c >>> +++ b/hw/pci/pci.c >>> @@ -2264,15 +2264,13 @@ static void pci_del_option_rom(PCIDevice *pdev) >>> * in pci config space >>> */ >>> int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, >>> - uint8_t offset, uint8_t size) >>> + uint8_t offset, uint8_t size, >>> + Error **errp) >>> { >>> int ret; >>> - Error *local_err = NULL; >>> >>> - ret = pci_add_capability2(pdev, cap_id, offset, size, &local_err); >>> - if (ret < 0) { >>> - error_report_err(local_err); >>> - } >>> + ret = pci_add_capability2(pdev, cap_id, offset, size, errp); >>> + >>> return ret; >>> } >> >> pci_add_capability() and pci_add_capability2() now do exactly the >> same, why are both being kept? I suggest replacing >> pci_add_capability2() with pci_add_capability() everywhere (on a >> separate patch). >> > > Completely remove pci_add_capability and direct use pci_add_capability2() > everywhere is it a more thorough way?
You're converting pci_add_capability() to Error because you need the Error for your conversions to realize(). I recommend to change the calls where you need the Error (and only these) to call pci_add_capability2() instead. When no calls to pci_add_capability() remain, we remove it. If that becomes the case in your series, you remove it. Okay?