[virt-tools-list] [PATCHv2 2/2] util: Fix the size of sorted_displays allocation

[Jonathon Jongsma]

Looks good to me. ACK both.

On Wed, 2015-10-21 at 16:08 +0200, Fabiano Fidêncio wrote:
> As sorted_displays is a vector containing all displays' order, its
> allocation size must be the maximum display id + 1 instead of the
> maximum display id. Also, fix the size used for sorting and iterating
> the sorted_displays vector.
> 
> Valgrind log:
> ==15946== Invalid write of size 4
> ==15946==    at 0x4169C0: virt_viewer_align_monitors_linear (virt
> -viewer-util.c:581)
> ==15946==    by 0x42248B:
> virt_viewer_session_on_monitor_geometry_changed (virt-viewer
> -session.c:438)
> ==15946==    by 0xBB41F03: _g_closure_invoke_va (gclosure.c:831)
> ==15946==    by 0xBB5BC7C: g_signal_emit_valist (gsignal.c:3214)
> ==15946==    by 0xBB5C764: g_signal_emit_by_name (gsignal.c:3401)
> ==15946==    by 0x4328F3:
> virt_viewer_display_spice_monitor_geometry_changed (virt-viewer
> -display-spice.c:93)
> ==15946==    by 0x432D60: virt_viewer_display_spice_size_allocate
> (virt-viewer-display-spice.c:224)
> ==15946==    by 0xBB41CD4: g_closure_invoke (gclosure.c:768)
> ==15946==    by 0xBB53538: signal_emit_unlocked_R (gsignal.c:3549)
> ==15946==    by 0xBB5BEEF: g_signal_emit_valist (gsignal.c:3305)
> ==15946==    by 0xBB5C29E: g_signal_emit (gsignal.c:3361)
> ==15946==    by 0x637D6F6: gtk_widget_size_allocate_with_baseline
> (gtkwidget.c:6093)
> ==15946==  Address 0x18c79d4c is 0 bytes after a block of size 12
> alloc'd
> ==15946==    at 0x4C2A9C7: calloc (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==15946==    by 0xBDD36D1: g_malloc0 (gmem.c:127)
> ==15946==    by 0x41698D: virt_viewer_align_monitors_linear (virt
> -viewer-util.c:577)
> ==15946==    by 0x42248B:
> virt_viewer_session_on_monitor_geometry_changed (virt-viewer
> -session.c:438)
> ==15946==    by 0xBB41F03: _g_closure_invoke_va (gclosure.c:831)
> ==15946==    by 0xBB5BC7C: g_signal_emit_valist (gsignal.c:3214)
> ==15946==    by 0xBB5C764: g_signal_emit_by_name (gsignal.c:3401)
> ==15946==    by 0x4328F3:
> virt_viewer_display_spice_monitor_geometry_changed (virt-viewer
> -display-spice.c:93)
> ==15946==    by 0x432D60: virt_viewer_display_spice_size_allocate
> (virt-viewer-display-spice.c:224)
> ==15946==    by 0xBB41CD4: g_closure_invoke (gclosure.c:768)
> ==15946==    by 0xBB53538: signal_emit_unlocked_R (gsignal.c:3549)
> ==15946==    by 0xBB5BEEF: g_signal_emit_valist (gsignal.c:3305)
> 
> Resolves: rhbz#1272650
> Related: rhbz#1267184
> ---
> Changes since v1:
> - Fix the size used for sorting and iterating the sorted_displays
> vector.
> ---
>  src/virt-viewer-util.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index e9f771b..f2ccd13 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -565,6 +565,7 @@ virt_viewer_align_monitors_linear(GHashTable
> *displays)
>      gint i, x = 0;
>      guint *sorted_displays;
>      guint max_id = 0;
> +    guint ndisplays = 0;
>      GHashTableIter iter;
>      gpointer key, value;
>  
> @@ -574,19 +575,21 @@ virt_viewer_align_monitors_linear(GHashTable
> *displays)
>          return;
>  
>      g_hash_table_foreach(displays, find_max_id, &max_id);
> -    sorted_displays = g_new0(guint, max_id);
> +    ndisplays = max_id + 1;
> +
> +    sorted_displays = g_new0(guint, ndisplays);
>  
>      g_hash_table_iter_init(&iter, displays);
>      while (g_hash_table_iter_next(&iter, &key, &value))
>          sorted_displays[GPOINTER_TO_INT(key)] =
> GPOINTER_TO_INT(key);
>  
> -    g_qsort_with_data(sorted_displays, max_id, sizeof(guint),
> displays_cmp, displays);
> +    g_qsort_with_data(sorted_displays, ndisplays, sizeof(guint),
> displays_cmp, displays);
>  
>      /* adjust monitor positions so that there's no gaps or overlap
> between
>       * monitors */
> -    for (i = 0; i < max_id; i++) {
> +    for (i = 0; i < ndisplays; i++) {
>          guint nth = sorted_displays[i];
> -        g_assert(nth < max_id);
> +        g_assert(nth < ndisplays);
>          GdkRectangle *rect = g_hash_table_lookup(displays,
> GINT_TO_POINTER(nth));
>          rect->x = x;
>          rect->y = 0;