Re: [Xen-devel] [PATCH v4 8/8] ioreq-server: bring the PCI hotplug controller implementation into Xen

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 09 April 2014 14:34
> To: Paul Durrant
> Cc: Ian Campbell; Ian Jackson; Stefano Stabellini; xen-devel@xxxxxxxxxxxxx
> Subject: Re: [PATCH v4 8/8] ioreq-server: bring the PCI hotplug controller
> implementation into Xen
> 
> >>> On 02.04.14 at 17:11, <paul.durrant@xxxxxxxxxx> wrote:
> > +static int handle_gpe_io(
> > +    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
> > +{
> > +    struct vcpu *v = current;
> > +    struct domain *d = v->domain;
> > +    struct hvm_hotplug  *hp = &d->arch.hvm_domain.hotplug;
> > +
> > +    if ( bytes != 1 )
> 
> Is this really a valid restriction?
> 

Hmm, I believe so but I guess it would be more pragmatic to handle word and 
double-word access too.

> > +int gpe_init(struct domain *d)
> > +{
> > +    struct hvm_hotplug  *hp = &d->arch.hvm_domain.hotplug;
> > +
> > +    hp->gpe_sts = xzalloc_array(uint8_t, ACPI_GPE0_BLK_LEN_V1 / 2);
> > +    if ( hp->gpe_sts == NULL )
> > +        goto fail1;
> > +
> > +    hp->gpe_en = xzalloc_array(uint8_t, ACPI_GPE0_BLK_LEN_V1 / 2);
> > +    if ( hp->gpe_en == NULL )
> > +        goto fail2;
> 
> I'd like to ask (also elsewhere in this series) to try to limit the number
> of "goto"s to the absolute minimum required to help code readability.

Personally I find using forward jumps to fail labels the most readable form of 
error exit - I wish they were used more widely.

> There's no need for them here: Allocate both blocks, then check both
> pointers, and if either is NULL free them both in a single error path.
> 

I'll alloc a single array and carve it in half.

  Paul


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel