| Message ID | 20240418100433.1085447-4-zhao1.liu@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/cxl/cxl-cdat: Make cxl_doe_cdat_init() return boolean | expand |
On 18/4/24 12:04, Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > As error.h suggested, the best practice for callee is to return > something to indicate success / failure. > > With returned boolean, there's no need to dereference @errp to check > failure case. > > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > hw/cxl/cxl-cdat.c | 6 +++--- > hw/mem/cxl_type3.c | 3 +-- > hw/pci-bridge/cxl_upstream.c | 3 +-- > include/hw/cxl/cxl_component.h | 2 +- > 4 files changed, 6 insertions(+), 8 deletions(-) > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h > index 5012fab6f763..945ee6ffd045 100644 > --- a/include/hw/cxl/cxl_component.h > +++ b/include/hw/cxl/cxl_component.h > @@ -273,7 +273,7 @@ hwaddr cxl_decode_ig(int ig); > CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb); > bool cxl_get_hb_passthrough(PCIHostState *hb); > > -void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); > +bool cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); > void cxl_doe_cdat_release(CXLComponentState *cxl_cstate); > void cxl_doe_cdat_update(CXLComponentState *cxl_cstate, Error **errp); Another candidate ;)
Hi Philippe, On Thu, Apr 18, 2024 at 02:06:15PM +0200, Philippe Mathieu-Daudé wrote: [snip] > > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h > > index 5012fab6f763..945ee6ffd045 100644 > > --- a/include/hw/cxl/cxl_component.h > > +++ b/include/hw/cxl/cxl_component.h > > @@ -273,7 +273,7 @@ hwaddr cxl_decode_ig(int ig); > > CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb); > > bool cxl_get_hb_passthrough(PCIHostState *hb); > > -void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); > > +bool cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); > > void cxl_doe_cdat_release(CXLComponentState *cxl_cstate); > > void cxl_doe_cdat_update(CXLComponentState *cxl_cstate, Error **errp); > > Another candidate ;) > I guess you mean cxl_doe_cdat_update()? ;-) It's a special case since it has only one use case and in that case, &error_fatal is passed as @errp. So then it doesn't need to check the return value. Thanks, Zhao
diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c index e7bc1380bfbf..959a55518e65 100644 --- a/hw/cxl/cxl-cdat.c +++ b/hw/cxl/cxl-cdat.c @@ -189,14 +189,14 @@ static bool ct3_load_cdat(CDATObject *cdat, Error **errp) return true; } -void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp) +bool cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp) { CDATObject *cdat = &cxl_cstate->cdat; if (cdat->filename) { - ct3_load_cdat(cdat, errp); + return ct3_load_cdat(cdat, errp); } else { - ct3_build_cdat(cdat, errp); + return ct3_build_cdat(cdat, errp); } } diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index b0a7e9f11b64..3e42490b6ce8 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -705,8 +705,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) cxl_cstate->cdat.build_cdat_table = ct3_build_cdat_table; cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table; cxl_cstate->cdat.private = ct3d; - cxl_doe_cdat_init(cxl_cstate, errp); - if (*errp) { + if (!cxl_doe_cdat_init(cxl_cstate, errp)) { goto err_free_special_ops; } diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c index 783fa6adac19..e51221a5f334 100644 --- a/hw/pci-bridge/cxl_upstream.c +++ b/hw/pci-bridge/cxl_upstream.c @@ -338,8 +338,7 @@ static void cxl_usp_realize(PCIDevice *d, Error **errp) cxl_cstate->cdat.build_cdat_table = build_cdat_table; cxl_cstate->cdat.free_cdat_table = free_default_cdat_table; cxl_cstate->cdat.private = d; - cxl_doe_cdat_init(cxl_cstate, errp); - if (*errp) { + if (!cxl_doe_cdat_init(cxl_cstate, errp)) { goto err_cap; } diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index 5012fab6f763..945ee6ffd045 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -273,7 +273,7 @@ hwaddr cxl_decode_ig(int ig); CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb); bool cxl_get_hb_passthrough(PCIHostState *hb); -void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); +bool cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); void cxl_doe_cdat_release(CXLComponentState *cxl_cstate); void cxl_doe_cdat_update(CXLComponentState *cxl_cstate, Error **errp);