Re: [PATCH] x86/HVM: correct cleanup after failed viridian_vcpu_init()

["Durrant, Paul" <xadimgnik@xxxxxxxxx>]

On 18/10/2021 11:42, Ian Jackson wrote:
> Jan Beulich writes ("[PATCH] x86/HVM: correct cleanup after failed 
> viridian_vcpu_init()"):
> > This happens after nestedhvm_vcpu_initialise(), so its effects also need
> > to be undone.
> >
> > Fixes: 40a4a9d72d16 ("viridian: add init hooks")
> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -1583,7 +1583,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
> >   
> >       rc = viridian_vcpu_init(v);
> >       if ( rc )
> > -        goto fail5;
> > +        goto fail6;
> Not acomment about the patch; rather about the code in general.
>
> I have not looked at the context.
>
> But OMG, this is horrific.  How can anyone write code in such an idiom
> without writing endless bugs ?
Fairly easily. I think this is the first one due to an incorrect exit label.
Using such an idiom in the Windows PV drivers had meant many issues 
could be easily debugged without further code modification because you 
get an fairly instant audit trail of the route out of any failure condition.
  Paul