From 72692b11441e99b6b6dfd72de2f6f4113b901a02 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Mon, 7 Jan 2019 14:09:16 +0100 Subject: [PATCH] clutter-box-layout: Use floats and assert on denormal numbers `distribute_natural_allocation` expects an input >= 0 of type `gint`. In `get_preferred_size_for_opposite_orientation` it is used with an unchecked variable `size` of type `gfloat`, which in case it is `Infinity`, gets passed on in the macro `MAX (0, size)`. `Infinity` becomes `G_MININT` when implicitly casted to `gint` in `distribute_natural_allocation`, triggering the assertion `extra_space >= 0`. The resulting warning in the log is counter intuitive and not very helpful. Use `float` in `distribute_natural_allocation` instead of `gint` and assert on denormal values so we can more easily identify bugs. Additionally change some types while at it and add a even more expressive warning referencing the actor at one point. https://gitlab.gnome.org/GNOME/mutter/merge_requests/375 --- clutter/clutter/clutter-box-layout.c | 47 ++++++++++++++++++---------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/clutter/clutter/clutter-box-layout.c b/clutter/clutter/clutter-box-layout.c index 1e7cb7db5..a3197f43f 100644 --- a/clutter/clutter/clutter-box-layout.c +++ b/clutter/clutter/clutter-box-layout.c @@ -150,9 +150,9 @@ typedef struct _RequestedSize gfloat natural_size; } RequestedSize; -static gint distribute_natural_allocation (gint extra_space, - guint n_requested_sizes, - RequestedSize *sizes); +static float distribute_natural_allocation (float extra_space, + unsigned int n_requested_sizes, + RequestedSize *sizes); static void count_expand_children (ClutterLayoutManager *layout, ClutterContainer *container, gint *visible_children, @@ -624,7 +624,19 @@ get_preferred_size_for_opposite_orientation (ClutterBoxLayout *self, else { /* Bring children up to size first */ - size = distribute_natural_allocation (MAX (0, size), nvis_children, sizes); + if (isnormal (size) || size == 0) + { + size = distribute_natural_allocation (MAX (0, size), + nvis_children, + sizes); + } + else + { + g_critical ("Actor %s (%p) received the invalid " + "value %f as minimum/natural size\n", + G_OBJECT_TYPE_NAME (container), container, size); + size = 0; + } /* Calculate space which hasn't distributed yet, * and is available for expanding children. @@ -879,17 +891,18 @@ compare_gap (gconstpointer p1, * * Pulled from gtksizerequest.c from Gtk+ */ -static gint -distribute_natural_allocation (gint extra_space, - guint n_requested_sizes, +static float +distribute_natural_allocation (float extra_space, + unsigned int n_requested_sizes, RequestedSize *sizes) { - guint *spreading; - gint i; + unsigned int *spreading; + int i; + g_return_val_if_fail (isnormal (extra_space) || extra_space == 0, 0); g_return_val_if_fail (extra_space >= 0, 0); - spreading = g_newa (guint, n_requested_sizes); + spreading = g_newa (unsigned int, n_requested_sizes); for (i = 0; i < n_requested_sizes; i++) spreading[i] = i; @@ -913,7 +926,7 @@ distribute_natural_allocation (gint extra_space, /* Sort descending by gap and position. */ g_qsort_with_data (spreading, - n_requested_sizes, sizeof (guint), + n_requested_sizes, sizeof (unsigned int), compare_gap, sizes); /* Distribute available space. @@ -925,11 +938,11 @@ distribute_natural_allocation (gint extra_space, * Sort order and reducing remaining space by assigned space * ensures that space is distributed equally. */ - gint glue = (extra_space + i) / (i + 1); - gint gap = sizes[(spreading[i])].natural_size - - sizes[(spreading[i])].minimum_size; + int glue = (extra_space + i) / (i + 1); + int gap = sizes[(spreading[i])].natural_size + - sizes[(spreading[i])].minimum_size; - gint extra = MIN (glue, gap); + int extra = MIN (glue, gap); sizes[spreading[i]].minimum_size += extra; @@ -1056,7 +1069,9 @@ clutter_box_layout_allocate (ClutterLayoutManager *layout, else { /* Bring children up to size first */ - size = distribute_natural_allocation (MAX (0, size), nvis_children, sizes); + size = (gint) distribute_natural_allocation (MAX (0, (float) size), + nvis_children, + sizes); /* Calculate space which hasn't distributed yet, * and is available for expanding children.