[gnome-software/wip/hughsie/histogram-crash] Fix crash when viewing application details



commit 236a05e16c5880e47b1e52a3ece2f9d77b628b49
Author: Richard Hughes <richard hughsie com>
Date:   Fri Feb 21 13:42:55 2020 +0000

    Fix crash when viewing application details
    
    The code was creating an array of guint32 and reading back an array of gint
    which caused a crazy histogram and an invalid read in some circumstances.
    
    Standardize to reading and writing a guint32 everywhere and make the code
    somewhat simpler.
    
    Fixes https://gitlab.gnome.org/GNOME/gnome-software/issues/928

 lib/gs-app.c              |  8 +++----
 src/gs-details-page.c     |  4 ++--
 src/gs-review-histogram.c | 53 ++++++++++++++++++++++++++---------------------
 3 files changed, 35 insertions(+), 30 deletions(-)
---
diff --git a/lib/gs-app.c b/lib/gs-app.c
index b7830009..5bc93fd1 100644
--- a/lib/gs-app.c
+++ b/lib/gs-app.c
@@ -580,8 +580,8 @@ gs_app_to_string_append (GsApp *app, GString *str)
                gs_app_kv_printf (str, "rating", "%i", priv->rating);
        if (priv->review_ratings != NULL) {
                for (i = 0; i < priv->review_ratings->len; i++) {
-                       gint rat = g_array_index (priv->review_ratings, gint, i);
-                       gs_app_kv_printf (str, "review-rating", "[%u:%i]",
+                       guint32 rat = g_array_index (priv->review_ratings, guint32, i);
+                       gs_app_kv_printf (str, "review-rating", "[%u:%u]",
                                          i, rat);
                }
        }
@@ -2881,7 +2881,7 @@ gs_app_set_rating (GsApp *app, gint rating)
  *
  * Gets the review ratings.
  *
- * Returns: (element-type gint) (transfer none): a list
+ * Returns: (element-type guint32) (transfer none): a list
  *
  * Since: 3.22
  **/
@@ -2896,7 +2896,7 @@ gs_app_get_review_ratings (GsApp *app)
 /**
  * gs_app_set_review_ratings:
  * @app: a #GsApp
- * @review_ratings: (element-type gint): a list
+ * @review_ratings: (element-type guint32): a list
  *
  * Sets the review ratings.
  *
diff --git a/src/gs-details-page.c b/src/gs-details-page.c
index 4dd88a1e..8eb07bd3 100644
--- a/src/gs-details-page.c
+++ b/src/gs-details-page.c
@@ -1565,7 +1565,7 @@ gs_details_page_refresh_reviews (GsDetailsPage *self)
                }
                if (review_ratings != NULL) {
                        for (i = 0; i < review_ratings->len; i++)
-                               n_reviews += (guint) g_array_index (review_ratings, gint, i);
+                               n_reviews += (guint) g_array_index (review_ratings, guint32, i);
                } else if (gs_app_get_reviews (self->app) != NULL) {
                        n_reviews = gs_app_get_reviews (self->app)->len;
                }
@@ -1574,7 +1574,7 @@ gs_details_page_refresh_reviews (GsDetailsPage *self)
        /* enable appropriate widgets */
        gtk_widget_set_visible (self->star, show_reviews);
        gtk_widget_set_visible (self->box_reviews, show_reviews);
-       gtk_widget_set_visible (self->histogram, review_ratings != NULL);
+       gtk_widget_set_visible (self->histogram, review_ratings != NULL && review_ratings->len > 0);
        gtk_widget_set_visible (self->label_review_count, n_reviews > 0);
 
        /* update the review label next to the star widget */
diff --git a/src/gs-review-histogram.c b/src/gs-review-histogram.c
index bbd297c8..21455f55 100644
--- a/src/gs-review-histogram.c
+++ b/src/gs-review-histogram.c
@@ -39,36 +39,41 @@ void
 gs_review_histogram_set_ratings (GsReviewHistogram *histogram,
                                 GArray *review_ratings)
 {
-       GsReviewHistogramPrivate *priv;
-       gdouble max;
-       gint count[5] = { 0, 0, 0, 0, 0 };
-       guint i;
+       GsReviewHistogramPrivate *priv = gs_review_histogram_get_instance_private (histogram);
+       gdouble fraction[6] = { 0.0f };
+       guint32 max = 0;
+       guint32 total = 0;
 
        g_return_if_fail (GS_IS_REVIEW_HISTOGRAM (histogram));
-       priv = gs_review_histogram_get_instance_private (histogram);
 
-       /* Scale to maximum value */
-       for (max = 0, i = 0; i < review_ratings->len; i++) {
-               gint c;
+       /* sanity check */
+       if (review_ratings->len != 6) {
+               g_warning ("ratings data incorrect expected 012345");
+               return;
+       }
 
-               c = g_array_index (review_ratings, gint, i);
-               if (c > max)
-                       max = c;
-               if (i > 0 && i < 6)
-                       count[i - 1] = c;
+       /* idx 0 is '0 stars' which we don't support in the UI */
+       for (guint i = 1; i < review_ratings->len; i++) {
+               guint32 c = g_array_index (review_ratings, guint32, i);
+               max = MAX (c, max);
+       }
+       for (guint i = 1; i < review_ratings->len; i++) {
+               guint32 c = g_array_index (review_ratings, guint32, i);
+               fraction[i] = max > 0 ? (gdouble) c / (gdouble) max : 0.f;
+               total += c;
        }
 
-       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar5), count[4] / max);
-       set_label (priv->label_count5, count[4]);
-       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar4), count[3] / max);
-       set_label (priv->label_count4, count[3]);
-       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar3), count[2] / max);
-       set_label (priv->label_count3, count[2]);
-       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar2), count[1] / max);
-       set_label (priv->label_count2, count[1]);
-       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar1), count[0] / max);
-       set_label (priv->label_count1, count[0]);
-       set_label (priv->label_total, count[0] + count[1] + count[2] + count[3] + count[4]);
+       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar5), fraction[5]);
+       set_label (priv->label_count5, g_array_index (review_ratings, guint, 5));
+       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar4), fraction[4]);
+       set_label (priv->label_count4, g_array_index (review_ratings, guint, 4));
+       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar3), fraction[3]);
+       set_label (priv->label_count3, g_array_index (review_ratings, guint, 3));
+       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar2), fraction[2]);
+       set_label (priv->label_count2, g_array_index (review_ratings, guint, 2));
+       gs_review_bar_set_fraction (GS_REVIEW_BAR (priv->bar1), fraction[1]);
+       set_label (priv->label_count1, g_array_index (review_ratings, guint, 1));
+       set_label (priv->label_total, total);
 }
 
 static void


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]