[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 6/6] xen/domain: Use IS_ERR_OR_NULL() when checking the return value of domain_create()
On Wed, Feb 28, 2018 at 2:14 PM, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote: > This means that hitting the fail path with with err set to 0 isn't considered > as success by the callers. All current codepaths look fine. > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > --- > CC: Jan Beulich <JBeulich@xxxxxxxx> > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx> > CC: Julien Grall <julien.grall@xxxxxxx> > --- > xen/arch/arm/mm.c | 6 +++--- > xen/arch/arm/setup.c | 2 +- > xen/arch/x86/mm.c | 6 +++--- > xen/arch/x86/setup.c | 2 +- > xen/common/domctl.c | 2 +- > xen/common/schedule.c | 2 +- > 6 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 3c328e2..16a08cf 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -521,7 +521,7 @@ void __init arch_init_memory(void) > * their domain field set to dom_xen. > */ > dom_xen = domain_create(DOMID_XEN, DOMCRF_dummy, 0, NULL); > - BUG_ON(IS_ERR(dom_xen)); > + BUG_ON(IS_ERR_OR_NULL(dom_xen)); > > /* > * Initialise our DOMID_IO domain. > @@ -529,14 +529,14 @@ void __init arch_init_memory(void) > * array. Mappings occur at the priv of the caller. > */ > dom_io = domain_create(DOMID_IO, DOMCRF_dummy, 0, NULL); > - BUG_ON(IS_ERR(dom_io)); > + BUG_ON(IS_ERR_OR_NULL(dom_io)); > > /* > * Initialise our COW domain. > * This domain owns sharable pages. > */ > dom_cow = domain_create(DOMID_COW, DOMCRF_dummy, 0, NULL); > - BUG_ON(IS_ERR(dom_cow)); > + BUG_ON(IS_ERR_OR_NULL(dom_cow)); > } > > static inline lpae_t pte_of_xenaddr(vaddr_t va) > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 032a6a8..6646074 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -857,7 +857,7 @@ void __init start_xen(unsigned long boot_phys_offset, > config.nr_spis = gic_number_lines() - 32; > > dom0 = domain_create(0, 0, 0, &config); > - if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) > + if ( IS_ERR_OR_NULL(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) > panic("Error creating domain 0"); > > dom0->is_privileged = 1; > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > index e1f089b..c44b781 100644 > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -272,7 +272,7 @@ void __init arch_init_memory(void) > * (but be [partly] controlled by Dom0 nevertheless). > */ > dom_xen = domain_create(DOMID_XEN, DOMCRF_dummy, 0, NULL); > - BUG_ON(IS_ERR(dom_xen)); > + BUG_ON(IS_ERR_OR_NULL(dom_xen)); > INIT_LIST_HEAD(&dom_xen->arch.pdev_list); > > /* > @@ -281,14 +281,14 @@ void __init arch_init_memory(void) > * array. Mappings occur at the priv of the caller. > */ > dom_io = domain_create(DOMID_IO, DOMCRF_dummy, 0, NULL); > - BUG_ON(IS_ERR(dom_io)); > + BUG_ON(IS_ERR_OR_NULL(dom_io)); > > /* > * Initialise our COW domain. > * This domain owns sharable pages. > */ > dom_cow = domain_create(DOMID_COW, DOMCRF_dummy, 0, NULL); > - BUG_ON(IS_ERR(dom_cow)); > + BUG_ON(IS_ERR_OR_NULL(dom_cow)); > > /* > * First 1MB of RAM is historically marked as I/O. If we booted PVH, > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index ac530ec..92428da 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1640,7 +1640,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) > > /* Create initial domain 0. */ > dom0 = domain_create(get_initial_domain_id(), domcr_flags, 0, &config); > - if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) > + if ( IS_ERR_OR_NULL(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) > panic("Error creating domain 0"); > > if ( !pv_shim ) > diff --git a/xen/common/domctl.c b/xen/common/domctl.c > index 50f7422..18dcb2d 100644 > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -547,7 +547,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) > u_domctl) > > d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref, > &op->u.createdomain.config); > - if ( IS_ERR(d) ) > + if ( IS_ERR_OR_NULL(d) ) > { > ret = PTR_ERR(d); > d = NULL; This will fail to crash if domain_create() returns NULL, but it will still return 0 from the hypercall (since AFAICT PTR_ERR(NULL) will be just '0'). Is that what we want? Given that anywhere else this returns NULL will cause a BUG() or a panic, I almost feel like a BUG_ON(d == NULL) would be more appropriate. Everything else looks good to me. -George _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |