Re: [Xen-devel] [PATCH 4/4] xen/hvc: Fix up checks when the info is allocated.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Jul 03, 2012 at 05:11:18PM +0100, Jan Beulich wrote:
> >>> On 03.07.12 at 17:40, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > Coverity would complain about this - even thought it looks OK.
> > 
> > CID 401957
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  drivers/tty/hvc/hvc_xen.c |   15 ++++++---------
> >  1 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> > index 944eaeb..fbf41be 100644
> > --- a/drivers/tty/hvc/hvc_xen.c
> > +++ b/drivers/tty/hvc/hvc_xen.c
> > @@ -209,10 +209,8 @@ static int xen_hvm_console_init(void)
> >  		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
> >  		if (!info)
> >  			return -ENOMEM;
> > -	}
> > -
> > -	/* already configured */
> > -	if (info->intf != NULL)
> > +	} else if (info->intf != NULL) {
> > +		/* already configured */
> >  		return 0;
> 
> Is this patch perhaps stale? There appears to be a missing } here,
> without which I can't see how this would have built, ...

<sighs> Yes. That is what I get for cherry-picking and in a hurry picking
the options.


> 
> >  	/*
> >  	 * If the toolstack (or the hypervisor) hasn't set these values, the
> > @@ -220,6 +218,7 @@ static int xen_hvm_console_init(void)
> >  	 * theoretically correct values, in practice they never are and they
> >  	 * mean that a legacy toolstack hasn't initialized the pv console correctly.
> >  	 */
> > +	 */
> 
> ... and this one likely wouldn't build either.

Heh. It certainly does not. The "real" patch has that fix.
> 
> Jan
> 
> >  	r = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN, &v);
> >  	if (r < 0 || v == 0)
> >  		goto err;
> > @@ -259,12 +258,10 @@ static int xen_pv_console_init(void)
> >  		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
> >  		if (!info)
> >  			return -ENOMEM;
> > -	}
> > -
> > -	/* already configured */
> > -	if (info->intf != NULL)
> > +	} else if (info->intf != NULL) {
> > +		/* already configured */
> >  		return 0;
> > -
> > +	}
> >  	info->evtchn = xen_start_info->console.domU.evtchn;
> >  	info->intf = mfn_to_virt(xen_start_info->console.domU.mfn);
> >  	info->vtermno = HVC_COOKIE;
>