Re: [PATCH 06/12] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月27日周四 下午3:06写道：
>
>
>
> On 2020/8/26 20:20, Li Qiang wrote:
> > Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:15写道：
> >>
> >> Receiving error in local variable err, and forgot to free it.
> >> Considering that there is no place to deal with it. Clean up.
> >>
> >> Reported-by: Euler Robot <euler.robot@huawei.com>
> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> >> ---
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> ---
> >>  ui/gtk-gl-area.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> >> index 85f9d14c51..c740a7eb14 100644
> >> --- a/ui/gtk-gl-area.c
> >> +++ b/ui/gtk-gl-area.c
> >> @@ -142,15 +142,14 @@ QEMUGLContext 
> >> gd_gl_area_create_context(DisplayChangeListener *dcl,
> >>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >>      GdkWindow *window;
> >>      GdkGLContext *ctx;
> >> -    GError *err = NULL;
> >>
> >>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
> >>      window = gtk_widget_get_window(vc->gfx.drawing_area);
> >> -    ctx = gdk_window_create_gl_context(window, &err);
> >> +    ctx = gdk_window_create_gl_context(window, NULL);
> >>      gdk_gl_context_set_required_version(ctx,
> >>                                          params->major_ver,
> >>                                          params->minor_ver);
> >> -    gdk_gl_context_realize(ctx, &err);
> >> +    gdk_gl_context_realize(ctx, NULL);
> >>      return ctx;
> >>  }
> >
> > Maybe we should check the return value of  'gdk_window_create_gl_context'
> > and 'gdk_gl_context_realize' instead of omitting it?
>
> OK, Agree with you.
>
> How about check the value like the below?

I think it is OK.

> (Return NULL when error happens in gdk_gl_context_realize. It's different 
> from the original.)

Don't familiar with the internal of how gtk-gl work.

Maybe you can wait for other review or Gerd's decision.

Thanks,
Li Qiang

>
> Thanks.
>
> --------
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 85f9d14c51..98c22d23f5 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -147,10 +147,21 @@ QEMUGLContext 
> gd_gl_area_create_context(DisplayChangeListener *dcl,
>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>      window = gtk_widget_get_window(vc->gfx.drawing_area);
>      ctx = gdk_window_create_gl_context(window, &err);
> +    if (err) {
> +        g_printerr("Create gdk gl context failed: %s\n", err->message);
> +        g_error_free(err);
> +        return NULL;
> +    }
>      gdk_gl_context_set_required_version(ctx,
>                                          params->major_ver,
>                                          params->minor_ver);
>      gdk_gl_context_realize(ctx, &err);
> +    if (err) {
> +        g_printerr("Realize gdk gl context failed: %s\n", err->message);
> +        g_error_free(err);
> +        g_clear_object(&ctx);
> +        return NULL;
> +    }
>      return ctx;
>  }
>
>
> >
> > Thanks,
> > Li Qiang
> >
> >>
> >
> >> --
> >> 2.18.2
> >>
> >>
> > .
> >
>