[3/4] graphic_console_init: do not receive unneeded error descriptions

Error set by error_set is dynamically allocated and needs to be cleared
properly later.  graphic_console_init neither needs error descriptions nor frees
them.  Pass NULL instead of actual pointers to avoid unnecessary memory
allocations.

Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
---
 ui/console.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Am 18.04.2014 15:41, schrieb Kirill Batuzov:
> Error set by error_set is dynamically allocated and needs to be cleared
> properly later.  graphic_console_init neither needs error descriptions nor frees
> them.  Pass NULL instead of actual pointers to avoid unnecessary memory
> allocations.
> 
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> ---
>  ui/console.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/ui/console.c b/ui/console.c
> index e057755..438b6bd 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1590,7 +1590,6 @@ QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
>                                    const GraphicHwOps *hw_ops,
>                                    void *opaque)
>  {
> -    Error *local_err = NULL;
>      int width = 640;
>      int height = 480;
>      QemuConsole *s;
> @@ -1602,10 +1601,8 @@ QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
>      s->hw_ops = hw_ops;
>      s->hw = opaque;
>      if (dev) {
> -        object_property_set_link(OBJECT(s), OBJECT(dev),
> -                                 "device", &local_err);
> -        object_property_set_int(OBJECT(s), head,
> -                                "head", &local_err);
> +        object_property_set_link(OBJECT(s), OBJECT(dev), "device", NULL);
> +        object_property_set_int(OBJECT(s), head, "head", NULL);

I guess it would be better to use &error_abort rather than failing
silently. CC'ing Gerd.

Regards,
Andreas

>      }
>  
>      s->surface = qemu_create_displaysurface(width, height);

> >     if (dev) {
> > -        object_property_set_link(OBJECT(s), OBJECT(dev),
> > -                                 "device", &local_err);
> > -        object_property_set_int(OBJECT(s), head,
> > -                                "head", &local_err);
> > +        object_property_set_link(OBJECT(s), OBJECT(dev), "device", NULL);
> > +        object_property_set_int(OBJECT(s), head, "head", NULL);
> 
> I guess it would be better to use &error_abort rather than failing
> silently.

Agree.

cheers,
  Gerd

On Tue, 22 Apr 2014, Gerd Hoffmann wrote:

> > >     if (dev) {
> > > -        object_property_set_link(OBJECT(s), OBJECT(dev),
> > > -                                 "device", &local_err);
> > > -        object_property_set_int(OBJECT(s), head,
> > > -                                "head", &local_err);
> > > +        object_property_set_link(OBJECT(s), OBJECT(dev), "device", NULL);
> > > +        object_property_set_int(OBJECT(s), head, "head", NULL);
> > 
> > I guess it would be better to use &error_abort rather than failing
> > silently.
> 
> Agree.
>

The result of this change was a bit unexpected. QEMU fails to start
with the error "Insufficient permission to perform this operation". This
happens because "head" is a read-only property. Fortunately it is
usually (always?) zero at creation time and we always try to write zero
also. So it works somehow. But non-zero head will not work.

We need either to initialize it in new_console with right head value or
to change it to simple struct field. It depends entirely on our future
plans for it.

In v2 I also plan to change all occurrences of &local_err to &error_abort
in ui/console.c They all are unchecked and can cause memory leaks and/or
weird silent failures like setting of "head" property had.

Hi,

> We need either to initialize it in new_console with right head value or
> to change it to simple struct field. It depends entirely on our future
> plans for it.

It'll never change after initialization, so keeping it read-only and
initialize in new_console sounds good to me.

cheers,
  Gerd