Fix out of bounds access in get_bar, improve satiety_bar

CleverRaven · Oct 3, 2020 · 44d4315 · 44d4315
1 parent f70482e
commit 44d4315
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 17 deletions.
diff --git a/src/output.cpp b/src/output.cpp
@@ -1841,10 +1841,17 @@ get_bar( float cur, float max, int width, bool extra_resolution,
     status = status < 0 ? 0 : status;
     float sw = status * width;
 
-    nc_color col = colors[static_cast<int>( ( 1 - status ) * colors.size() )];
-    if( status == 0 ) {
-        col = colors.back();
-    } else if( ( sw < 0.5 ) && ( sw > 0 ) ) {
+    nc_color col;
+    if( !std::isfinite( status ) || colors.empty() ) {
+        col = c_red_red;
+    } else {
+        int ind = static_cast<int>( ( 1 - status ) * colors.size() );
+        ind = clamp<int>( ind, 0, colors.size() - 1 );
+        col = colors[ind];
+    }
+    if( !std::isfinite( sw ) || sw <= 0 ) {
+        result.clear();
+    } else if( sw < 0.5 ) {
         result = ":";
     } else {
         result += std::string( sw, '|' );
@@ -1969,16 +1976,19 @@ void insert_table( const catacurses::window &w, int pad, int line, int columns,
 std::string satiety_bar( const int calpereffv )
 {
     // Arbitrary max value we will cap our vague display to. Will be lower than the actual max value, but scaling fixes that.
-    constexpr int max_cal_per_effective_vol = 1500;
-    //Scaling the values.
-    const int scaled_max = std::sqrt( max_cal_per_effective_vol ) / 4;
-    const int scaled_cal = std::sqrt( calpereffv ) / 4;
+    constexpr float max_cal_per_effective_vol = 1500.0f;
+    // Scaling the values.
+    const float scaled_max = std::sqrt( max_cal_per_effective_vol );
+    const float scaled_cal = std::sqrt( calpereffv );
     const std::pair<std::string, nc_color> nourishment_bar = get_bar(
                 scaled_cal, scaled_max, 5, true );
     // Colorize the bar.
     std::string result = colorize( nourishment_bar.first, nourishment_bar.second );
     // Pad to 5 characters with dots.
-    result += std::string( 5 - nourishment_bar.first.length(), '.' );
+    const int width = utf8_width( nourishment_bar.first );
+    if( width < 5 ) {
+        result += std::string( 5 - width, '.' );
+    }
     return result;
 }
 

diff --git a/src/output.h b/src/output.h
@@ -597,8 +597,6 @@ std::string shortcut_text( nc_color shortcut_color, const std::string &fmt );
  * @param extra_resolution Double the resolution of displayed values with \ symbols.
  * @param colors A vector containing N colors with which to color the bar at different values
  */
-// The last color is used for an empty bar
-// extra_resolution
 std::pair<std::string, nc_color> get_bar( float cur, float max, int width = 5,
         bool extra_resolution = true,
         const std::vector<nc_color> &colors = { c_green, c_light_green, c_yellow, c_light_red, c_red } );

diff --git a/tests/comestible_test.cpp b/tests/comestible_test.cpp
@@ -207,8 +207,7 @@ TEST_CASE( "effective food volume and satiety", "[character][food][satiety]" )
     // If kcal per gram < 1.0, return 1.0
     CHECK( u.compute_effective_food_volume_ratio( apple ) == Approx( 1.0f ).margin( 0.01f ) );
     CHECK( u.compute_calories_per_effective_volume( apple ) == 396 );
-    // NOLINTNEXTLINE(cata-text-style): verbatim ellipses necessary for validation
-    CHECK( satiety_bar( 396 ) == "<color_c_yellow>||</color>..." );
+    CHECK( satiety_bar( 396 ) == "<color_c_yellow>||\\</color>.." );
 
     // Egg: 80 kcal / 40 g (1 serving)
     const item egg( "test_egg" );
@@ -220,7 +219,7 @@ TEST_CASE( "effective food volume and satiety", "[character][food][satiety]" )
     // If kcal per gram > 1.0 but less than 3.0, return ( kcal / gram )
     CHECK( u.compute_effective_food_volume_ratio( egg ) == Approx( 2.0f ).margin( 0.01f ) );
     CHECK( u.compute_calories_per_effective_volume( egg ) == 1333 );
-    CHECK( satiety_bar( 1333 ) == "<color_c_green>|||||</color>" );
+    CHECK( satiety_bar( 1333 ) == "<color_c_green>||||\\</color>" );
 
     // Pine nuts: 202 kcal / 30 g (4 servings)
     const item nuts( "test_pine_nuts" );
@@ -245,6 +244,10 @@ TEST_CASE( "food satiety bar", "[character][food][satiety]" )
     // NOLINTNEXTLINE(cata-text-style): verbatim ellipses necessary for validation
     CHECK( satiety_bar( 0 ) == "<color_c_red></color>....." );
     // NOLINTNEXTLINE(cata-text-style): verbatim ellipses necessary for validation
+    CHECK( satiety_bar( 1 ) == "<color_c_red>:</color>...." );
+    // NOLINTNEXTLINE(cata-text-style): verbatim ellipses necessary for validation
+    CHECK( satiety_bar( 50 ) == "<color_c_red>\\</color>...." );
+    // NOLINTNEXTLINE(cata-text-style): verbatim ellipses necessary for validation
     CHECK( satiety_bar( 100 ) == "<color_c_light_red>|</color>...." );
     // NOLINTNEXTLINE(cata-text-style): verbatim ellipses necessary for validation
     CHECK( satiety_bar( 200 ) == "<color_c_light_red>|\\</color>..." );
@@ -256,10 +259,10 @@ TEST_CASE( "food satiety bar", "[character][food][satiety]" )
     CHECK( satiety_bar( 700 ) == "<color_c_light_green>|||</color>.." );
     CHECK( satiety_bar( 800 ) == "<color_c_light_green>|||\\</color>." );
     CHECK( satiety_bar( 900 ) == "<color_c_light_green>|||\\</color>." );
-    CHECK( satiety_bar( 1000 ) == "<color_c_light_green>|||\\</color>." );
+    CHECK( satiety_bar( 1000 ) == "<color_c_green>||||</color>." );
     CHECK( satiety_bar( 1100 ) == "<color_c_green>||||</color>." );
     CHECK( satiety_bar( 1200 ) == "<color_c_green>||||</color>." );
-    CHECK( satiety_bar( 1300 ) == "<color_c_green>|||||</color>" );
-    CHECK( satiety_bar( 1400 ) == "<color_c_green>|||||</color>" );
+    CHECK( satiety_bar( 1300 ) == "<color_c_green>||||\\</color>" );
+    CHECK( satiety_bar( 1400 ) == "<color_c_green>||||\\</color>" );
     CHECK( satiety_bar( 1500 ) == "<color_c_green>|||||</color>" );
 }