[libvirt] [PATCH]: Fixed minor bugs in display and added OOM checks.

[Chris Lalancette]

Pritesh Kothari wrote:
> Hi All,
> 
> There was a minor bug in selecting the graphics type. if the graphics type was 
> desktop it was assumed that display is set for it, and thus crashed on strdup,
> so now checking if display is present before setting it.
> 
> The second bug was while setting the 3d acceleration parameter, VIR_ALLOC()
> returns 0 or greater if success and not the other way round.
> 
> Lastly added OOM checks in few places which were missed earlier.
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index edcb39b..885908c 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -1838,13 +1838,16 @@ static char *vboxDomainDumpXML(virDomainPtr dom, int flags) {
>                              def->videos[0]->type            = VIR_DOMAIN_VIDEO_TYPE_VBOX;
>                              def->videos[0]->vram            = VRAMSize;
>                              def->videos[0]->heads           = monitorCount;
> -                            if (VIR_ALLOC(def->videos[0]->accel)) {
> +                            if (VIR_ALLOC(def->videos[0]->accel) >= 0) {
>                                  def->videos[0]->accel->support3d = accelerate3DEnabled;
>                                  /* Not supported yet, but should be in 3.1 soon */
>                                  def->videos[0]->accel->support2d = 0;
> -                            }
> -                        }
> -                    }
> +                            } else
> +                                virReportOOMError(dom->conn);
> +                        } else
> +                            virReportOOMError(dom->conn);
> +                    } else
> +                        virReportOOMError(dom->conn);
>                  }

To be honest, I find this whole section of code difficult to read.  The problem
is that the "checking for success" on VIR_ALLOC() makes it so that the code you
care about is always heavily indented, and leads to the triple else block you
have above.  I much prefer the style used in the rest of libvirt, where you
check for failure and get out, leaving the code you care about not indented so
heavily.  The same general comment goes for the much of the rest of the code
below; while you are adding a bunch of (valuable) checks here, the style and
indentation makes it hard to follow.

-- 
Chris Lalancette