Stop using atoi/atof in item.cpp

Port to try_parse_integer and strtod.
anothersimulacrum · Jul 3, 2021 · 6f87018 · 6f87018
1 parent 011d5ef
commit 6f87018
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 7 deletions.
diff --git a/src/item.cpp b/src/item.cpp
@@ -95,6 +95,7 @@
 #include "string_id_utils.h"
 #include "text_snippets.h"
 #include "translations.h"
+#include "try_parse_integer.h"
 #include "units.h"
 #include "units_fwd.h"
 #include "units_utility.h"
@@ -1185,7 +1186,19 @@ double item::get_var( const std::string &name, const double default_value ) cons
     if( it == item_vars.end() ) {
         return default_value;
     }
-    return atof( it->second.c_str() );
+    const std::string &val = it->second;
+    char *end;
+    errno = 0;
+    double result = strtod( &val[0], &end );
+    if( errno != 0 ) {
+        debugmsg( "Error parsing floating point value from %s in item::get_var: %s",
+                  val, strerror( errno ) );
+        return default_value;
+    }
+    if( end != &val[0] + val.size() ) {
+        debugmsg( "Stray characters at end of floating point value %s in item::get_var", val );
+    }
+    return result;
 }
 
 void item::set_var( const std::string &name, const tripoint &value )
@@ -1200,9 +1213,19 @@ tripoint item::get_var( const std::string &name, const tripoint &default_value )
         return default_value;
     }
     std::vector<std::string> values = string_split( it->second, ',' );
-    return tripoint( atoi( values[0].c_str() ),
-                     atoi( values[1].c_str() ),
-                     atoi( values[2].c_str() ) );
+    cata_assert( values.size() == 3 );
+    auto convert_or_error = []( const std::string & s ) {
+        ret_val<int> result = try_parse_integer<int>( s, false );
+        if( result.success() ) {
+            return result.value();
+        } else {
+            debugmsg( "Error parsing tripoint coordinate in item::get_var: %s", result.str() );
+            return 0;
+        }
+    };
+    return tripoint( convert_or_error( values[0] ),
+                     convert_or_error( values[1] ),
+                     convert_or_error( values[2] ) );
 }
 
 void item::set_var( const std::string &name, const std::string &value )
@@ -2148,9 +2171,15 @@ void item::ammo_info( std::vector<iteminfo> &info, const iteminfo_query *parts,
     }
     if( parts->test( iteminfo_parts::AMMO_FX_RECOVER ) ) {
         for( const std::string &effect : ammo.ammo_effects ) {
-            if( effect.compare( 0, 8, "RECOVER_" ) == 0 ) {
-                int recover_chance;
-                sscanf( effect.c_str(), "RECOVER_%i", &recover_chance );
+            if( string_starts_with( effect, "RECOVER_" ) ) {
+                ret_val<int> try_recover_chance =
+                    try_parse_integer<int>( effect.substr( 8 ), false );
+                if( !try_recover_chance.success() ) {
+                    debugmsg( "Error parsing ammo RECOVER_ denominator: %s",
+                              try_recover_chance.str() );
+                    break;
+                }
+                int recover_chance = try_recover_chance.value();
                 if( recover_chance <= 5 ) {
                     fx.emplace_back( _( "Stands a <bad>very low</bad> chance of remaining intact once fired." ) );
                 } else if( recover_chance <= 10 ) {

diff --git a/tests/item_test.cpp b/tests/item_test.cpp
@@ -286,3 +286,14 @@ TEST_CASE( "items spawn in their default containers", "[item]" )
     check_spawning_in_container( "chem_black_powder" );
     check_spawning_in_container( "software_useless" );
 }
+
+TEST_CASE( "item variables round-trip accurately", "[item]" )
+{
+    item i( "water" );
+    i.set_var( "A", 17 );
+    CHECK( i.get_var( "A", 0 ) == 17 );
+    i.set_var( "B", 0.125 );
+    CHECK( i.get_var( "B", 0.0 ) == 0.125 );
+    i.set_var( "C", tripoint( 2, 3, 4 ) );
+    CHECK( i.get_var( "C", tripoint() ) == tripoint( 2, 3, 4 ) );
+}