Merge pull request #2270 from jamescowens/remove_stod

refactor: Replace local dependent string functions with non-locale versions in strencodings.h/cpp

Author: jamescowens

src/bignum.h

@@ -369,12 +369,11 @@ class CBigNum : public CBigNumBase
             psz++;
 
         // hex string to bignum
-        static const signed char phexdigit[256] = { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,1,2,3,4,5,6,7,8,9,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0xa,0xb,0xc,0xd,0xe,0xf,0,0,0,0,0,0,0,0,0 };
         *this = 0;
-        while (isxdigit(*psz))
+        while (HexDigit(*psz) >= 0)
         {
             *this <<= 4;
-            int n = phexdigit[(unsigned char)*psz++];
+            int n = HexDigit((unsigned char)*psz++);
             *this += n;
         }
         if (fNegative)

src/gridcoin/accrual/snapshot.h

@@ -552,11 +552,14 @@ class AccrualSnapshotFile
     //!
     static uint64_t ParseHeight(const fs::path& snapshot_path)
     {
-        try {
-            return std::stoull(snapshot_path.stem().string());
-        } catch (...) {
-            return 0;
+        uint64_t height = 0;
+
+        if (!ParseUInt64(snapshot_path.stem().string(), &height)) {
+            LogPrint(BCLog::LogFlags::SB, "WARN: %s: Filename in snapshot path does not contain a valid height number.",
+                     __func__);
         }
+
+        return height;
     }
 
     //!

src/gridcoin/researcher.cpp

@@ -87,7 +87,7 @@ bool UpdateRWSettingsForMode(const ResearcherMode mode, const std::string& email
 //!
 std::string LowerUnderscore(std::string data)
 {
-    boost::to_lower(data);
+    data = ToLower(data);
     boost::replace_all(data, "_", " ");
 
     return data;
@@ -304,7 +304,7 @@ std::set<std::string> GetTeamWhitelist()
             continue;
         }
 
-        boost::to_lower(team_name);
+        team_name = ToLower(team_name);
 
         teams.emplace(std::move(team_name));
     }
@@ -794,18 +794,23 @@ MiningProject::MiningProject(std::string name,
     , m_rac(std::move(rac))
     , m_error(Error::NONE)
 {
-    boost::to_lower(m_team);
+    m_team = ToLower(m_team);
 }
 
 MiningProject MiningProject::Parse(const std::string& xml)
 {
+    double rac = 0.0;
+
+    if (!ParseDouble(ExtractXML(xml, "<user_expavg_credit>", "</user_expavg_credit>"), &rac)) {
+        LogPrintf("WARN: %s: Unable to parse user RAC from legacy XML data.", __func__);
+    }
+
     MiningProject project(
         ExtractXML(xml, "<project_name>", "</project_name>"),
         Cpid::Parse(ExtractXML(xml, "<external_cpid>", "</external_cpid>")),
         ExtractXML(xml, "<team_name>", "</team_name>"),
         ExtractXML(xml, "<master_url>", "</master_url>"),
-        std::strtold(ExtractXML(xml, "<user_expavg_credit>",
-                                "</user_expavg_credit>").c_str(), nullptr));
+        rac);
 
     if (IsPoolCpid(project.m_cpid) && !gArgs.GetBoolArg("-pooloperator", false)) {
         project.m_error = MiningProject::Error::POOL;
@@ -1101,7 +1106,7 @@ std::string Researcher::Email()
     if (gArgs.GetBoolArg("-investor", false)) return email;
 
     email = gArgs.GetArg("-email", "");
-    boost::to_lower(email);
+    email = ToLower(email);
 
     return email;
 }
@@ -1348,7 +1353,7 @@ GRC::BeaconError Researcher::BeaconError() const
 
 bool Researcher::ChangeMode(const ResearcherMode mode, std::string email)
 {
-    boost::to_lower(email);
+    email = ToLower(email);
 
     if (mode == ResearcherMode::INVESTOR && ConfiguredForInvestorMode()) {
         return true;

src/gridcoin/scraper/http.cpp

@@ -171,10 +171,10 @@ namespace
         }
 
         constexpr char expected[] = "etag";
-        constexpr int32_t to_upper = 32;
+        constexpr int32_t shift_to_upper = 32;
 
         for (size_t i = 0; i < 4; ++i) {
-            if (header[i] != expected[i] && header[i] != expected[i] - to_upper) {
+            if (header[i] != expected[i] && header[i] != expected[i] - shift_to_upper) {
                 return std::string();
             }
         }

src/gridcoin/scraper/scraper.cpp

@@ -878,24 +878,59 @@ void ApplyCache(const std::string& key, T& result)
     try
     {
         if (std::is_same<T, double>::value)
-            result = stod(entry.value);
+        {
+            double out = 0.0;
+
+            if (!ParseDouble(entry.value, &out))
+            {
+                throw std::invalid_argument("Argument is not parseable as a double.");
+            }
+
+            // Have to do the copy here and below, because otherwise the compiler complains about putting &result directly
+            // above, since this is a template.
+            result = out;
+        }
         else if (std::is_same<T, int64_t>::value)
-            result = atoi64(entry.value);
+        {
+            int64_t out = 0;
+
+            if (!ParseInt64(entry.value, &out))
+            {
+                throw std::invalid_argument("Argument is not parseable as an int64_t.");
+            }
+
+            result = out;
+        }
         else if (std::is_same<T, unsigned int>::value)
-            // Throw away (zero out) negative integer
-            // This approach limits the range to the non-negative signed int, but that is good enough.
-            result = (unsigned int)std::max(0, stoi(entry.value));
+        {
+            unsigned int out = 0;
+
+            if (!ParseUInt(entry.value, &out))
+            {
+                throw std::invalid_argument("Argument is not parseable as an unsigned int.");
+            }
+
+            result = out;
+        }
         else if (std::is_same<T, bool>::value)
         {
             if (entry.value == "false" || entry.value == "0")
+            {
                 result = boost::lexical_cast<T>(false);
+            }
             else if (entry.value == "true" || entry.value == "1")
+            {
                 result = boost::lexical_cast<T>(true);
+            }
             else
+            {
                 throw std::invalid_argument("Argument not true or false");
+            }
         }
         else
+        {
             result = boost::lexical_cast<T>(entry.value);
+        }
     }
     catch (const std::exception&)
     {
@@ -1879,20 +1914,18 @@ bool ProcessProjectTeamFile(const std::string& project, const fs::path& file, co
 
             int64_t nTeamID = 0;
 
-            try
-            {
-                nTeamID = atoi64(sTeamID);
-            }
-            catch (const std::exception&)
+            if (!ParseInt64(sTeamID, &nTeamID))
             {
-                _log(logattribute::ERR, "ProccessProjectTeamFile", tfm::format("Ignoring bad team id for team %s.", sTeamName));
+                _log(logattribute::ERR, __func__, tfm::format("Ignoring bad team id for team %s.", sTeamName));
                 continue;
             }
 
             mTeamIdsForProject[sTeamName] = nTeamID;
         }
         else
+        {
             builder.append(line);
+        }
     }
 
     if (mTeamIdsForProject.empty())
@@ -2284,13 +2317,9 @@ bool ProcessProjectRacFileByCPID(const std::string& project, const fs::path& fil
                 const std::string& sTeamID = ExtractXML(data, "<teamid>", "</teamid>");
                 int64_t nTeamID = 0;
 
-                try
+                if (!ParseInt64(sTeamID, &nTeamID))
                 {
-                    nTeamID = atoi64(sTeamID);
-                }
-                catch (const std::exception&)
-                {
-                    _log(logattribute::ERR, "ProcessProjectRacFileByCPID", "Bad team id in user stats file data.");
+                    _log(logattribute::ERR, __func__, "Bad team id in user stats file data.");
                     continue;
                 }
 
@@ -2550,13 +2579,9 @@ bool LoadTeamIDList(const fs::path& file)
                 sProject = iter;
             else
             {
-                try
-                {
-                    nTeamID = atoi64(iter);
-                }
-                catch (std::exception&)
+                if (!ParseInt64(iter, &nTeamID))
                 {
-                    _log(logattribute::ERR, "LoadTeamIDList", "Ignoring invalid team id found in team id file.");
+                    _log(logattribute::ERR, __func__, "Ignoring invalid team id found in team id file.");
                     continue;
                 }
 
@@ -2898,7 +2923,16 @@ bool LoadScraperFileManifest(const fs::path& file)
         nhash.SetHex(vline[0].c_str());
         LoadEntry.hash = nhash;
 
-        LoadEntry.current = std::stoi(vline[1]);
+        // We will use the ParseUInt from strencodings to avoid the locale specific stoi.
+        unsigned int parsed_current = 0;
+
+        if (!ParseUInt(vline[1], &parsed_current))
+        {
+            _log(logattribute::ERR, __func__, "The \"current\" field not parsed correctly for a manifest entry. Skipping.");
+            continue;
+        }
+
+        LoadEntry.current = parsed_current;
 
         std::istringstream sstimestamp(vline[2]);
         sstimestamp >> ntimestamp;
@@ -2913,7 +2947,17 @@ bool LoadScraperFileManifest(const fs::path& file)
         {
             // Intended for explorer mode, where files not to be included in CScraperManifest
             // are to be maintained, such as team and host files.
-            LoadEntry.excludefromcsmanifest = std::stoi(vline[5]);
+            unsigned int parsed_exclude = 0;
+
+            if (!ParseUInt(vline[5], &parsed_exclude))
+            {
+                // This shouldn't happen given the conditional above, but to be thorough...
+                _log(logattribute::ERR, __func__, "The \"excludefromcsmanifest\" field not parsed correctly for a manifest "
+                                                  "entry. Skipping.");
+                continue;
+            }
+
+            LoadEntry.excludefromcsmanifest = parsed_exclude;
         }
         else
         {
@@ -3158,10 +3202,46 @@ bool ProcessProjectStatsFromStreamByCPID(const std::string& project, boostio::fi
         const std::string& sRAC = fields[2];
         const std::string& cpid = fields[3];
 
-        // Replace blank strings with zeros.
-        statsentry.statsvalue.dTC = (sTC.empty()) ? 0.0 : std::stod(sTC);
-        statsentry.statsvalue.dRAT = (sRAT.empty()) ? 0.0 : std::stod(sRAT);
-        statsentry.statsvalue.dRAC = (sRAC.empty()) ? 0.0 : std::stod(sRAC);
+        // Replace blank strings with zeros. Continue to next record with a logged error if there is a parsing failure.
+        if (sTC.empty())
+        {
+            statsentry.statsvalue.dTC = 0.0;
+        }
+        else
+        {
+            if (!ParseDouble(sTC, &statsentry.statsvalue.dTC))
+            {
+                _log(logattribute::ERR, __func__, "Cannot parse sTC " + sTC + " for cpid " + cpid);
+                continue;
+            }
+        }
+
+        if (sRAT.empty())
+        {
+            statsentry.statsvalue.dRAT = 0.0;
+        }
+        else
+        {
+            if (!ParseDouble(sRAT, &statsentry.statsvalue.dRAT))
+            {
+                _log(logattribute::ERR, __func__, "Cannot parse sRAT " + sRAT + " for cpid " + cpid);
+                continue;
+            }
+        }
+
+        if (sRAC.empty())
+        {
+            statsentry.statsvalue.dRAC = 0.0;
+        }
+        else
+        {
+            if (!ParseDouble(sRAC, &statsentry.statsvalue.dRAC))
+            {
+                _log(logattribute::ERR, __func__, "Cannot parse sRAC " + sRAC + " for cpid " + cpid);
+                continue;
+            }
+        }
+
         // At the individual (byCPIDbyProject) level the AvgRAC is the same as the RAC.
         statsentry.statsvalue.dAvgRAC = statsentry.statsvalue.dRAC;
         // Mag is dealt with on the second pass... so is left at 0.0 on the first pass.

src/gridcoin/staking/kernel.cpp

@@ -140,7 +140,7 @@ int64_t GetRSAWeightByBlock(const std::string& bb)
     constexpr size_t rsa_weight_offset = 13;
     constexpr size_t magnitude_offset = 15;
 
-    int64_t rsa_weight = 0;
+    int64_t rsa_weight_sum = 0;
 
     // General-purpose deserialization of claim contexts in the hashBoinc field
     // no longer parses out the RSA weight field, so we handle the special case
@@ -154,18 +154,24 @@ int64_t GetRSAWeightByBlock(const std::string& bb)
         if (n == cpid_offset && end - offset != 32) {
             return 0;
         } else if (n == rsa_weight_offset || n == magnitude_offset) {
-            rsa_weight += std::atoi(bb.substr(offset, end - offset).c_str());
+            int64_t rsa_weight = 0;
+
+            if (!ParseInt64(bb.substr(offset, end - offset), &rsa_weight)) {
+                error("%s: Unable to parse rsa weight from hashBoinc.", __func__);
+            }
+
+            rsa_weight_sum += rsa_weight;
         }
 
         offset = end + 3;
         end = bb.find("<|>", offset);
     }
 
-    if (rsa_weight < 0) {
+    if (rsa_weight_sum < 0) {
         return 0;
     }
 
-    return rsa_weight;
+    return rsa_weight_sum;
 }
 } // anonymous namespace
 

src/gridcoin/staking/reward.cpp

@@ -46,7 +46,12 @@ CAmount GRC::GetConstantBlockReward(const CBlockIndex* index)
     //TODO: refactor the expire checking to subroutine
     //Note: time constant is same as GetBeaconPublicKey
     if ((index->nTime - oCBReward.timestamp) <= (60 * 24 * 30 * 6 * 60)) {
-        reward = atoi64(oCBReward.value);
+        // This is a little slippery, because if we ever change CAmount from a int64_t, this will
+        // break. It is unlikely to ever change, however, and avoids an extra copy/implicit cast.
+        if (!ParseInt64(oCBReward.value, &reward)) {
+            error("%s: Cannot parse constant block reward from protocol entry: %s",
+                  __func__, oCBReward.value);
+        }
     }
 
     reward = std::clamp(reward, MIN_CBR, MAX_CBR);