Re: [virt-tools-list] [PATCH 1/2] session: Only create a hashtable if apply_monitor_geometry class exists

2015-10-21 Thread Jonathon Jongsma
ACK, but I'd like a couple changes to the commit log:

- in the summary: apply_monitor_geometry is a vfunc, not a class
- add a comment to the commit log explaining that this is to avoid a
memory leak when we return early.


On Wed, 2015-10-21 at 15:23 +0200, Fabiano FidĂȘncio wrote:
> Related: rhbz#1267184
> ---
>  src/virt-viewer-session.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
> index 2699f41..92ffd3f 100644
> --- a/src/virt-viewer-session.c
> +++ b/src/virt-viewer-session.c
> @@ -405,13 +405,15 @@
> virt_viewer_session_on_monitor_geometry_changed(VirtViewerSession*
> self,
>  VirtViewerSessionClass *klass;
>  gboolean all_fullscreen = TRUE;
>  /* GHashTable */
> -GHashTable *monitors = g_hash_table_new_full(g_direct_hash,
> g_direct_equal, NULL, g_free);
> +GHashTable *monitors;
>  GList *l;
>  
>  klass = VIRT_VIEWER_SESSION_GET_CLASS(self);
>  if (!klass->apply_monitor_geometry)
>  return;
>  
> +monitors = g_hash_table_new_full(g_direct_hash, g_direct_equal,
> NULL, g_free);
> +
>  for (l = self->priv->displays; l; l = l->next) {
>  VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l->data);
>  guint nth = 0;

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH 1/2] session: Only create a hashtable if apply_monitor_geometry class exists

2015-10-21 Thread Eduardo Lima (Etrunko)
ACK

Although I think you could add a description for this patch in the
message a memory leak on error path.

On 10/21/2015 11:23 AM, Fabiano FidĂȘncio wrote:
> Related: rhbz#1267184
> ---
>  src/virt-viewer-session.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
> index 2699f41..92ffd3f 100644
> --- a/src/virt-viewer-session.c
> +++ b/src/virt-viewer-session.c
> @@ -405,13 +405,15 @@ 
> virt_viewer_session_on_monitor_geometry_changed(VirtViewerSession* self,
>  VirtViewerSessionClass *klass;
>  gboolean all_fullscreen = TRUE;
>  /* GHashTable */
> -GHashTable *monitors = g_hash_table_new_full(g_direct_hash, 
> g_direct_equal, NULL, g_free);
> +GHashTable *monitors;
>  GList *l;
>  
>  klass = VIRT_VIEWER_SESSION_GET_CLASS(self);
>  if (!klass->apply_monitor_geometry)
>  return;
>  
> +monitors = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, 
> g_free);
> +
>  for (l = self->priv->displays; l; l = l->next) {
>  VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l->data);
>  guint nth = 0;
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list