[SERVER-8958] Replace strtod/strtol/atoi/atol/etc calls with parseNumberFromString<T>. Created: 12/Mar/13  Updated: 28/Mar/19  Resolved: 28/Mar/19

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Andy Schwerin Assignee: Andrew Morrow (Inactive)
Resolution: Won't Fix Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to SERVER-8936 TestParsingOverflow unit test failing Closed
Sprint: Dev Tools 2019-02-25, Dev Tools 2019-04-08
Participants:

 Description   
  • strtod on Windows cannot parse +/-Infinity or NaN.
  • Proper error checking is error prone for strtol and strtod, atoi, etc.

Exclude mongo/db/json.cpp (JSON parser), which depends on the integrated lex/parse behavior of strto*.



 Comments   
Comment by Andrew Morrow (Inactive) [ 28/Mar/19 ]

Despite discussion both on this ticket and on the PR, there just doesn't seem to be a lot of interest on spending time here.

Comment by Andrew Morrow (Inactive) [ 11/Mar/19 ]

Per internal discussion, the preference would be to converge on parseNumberFromString as a replacement, including for the std::sto family of functions.

Comment by Andrew Morrow (Inactive) [ 20/Feb/19 ]

Based on schwerin's search above, the number of remaining instances is now very low. Out of curiosity, I included the sto prefixed operations as well:

$ grep --exclude='json.*' --exclude='parse_number.*' --exclude="*vcx*" --exclude='src/mongo/gotools/*' -IrEn '(strto|ato|sto)[lid]' src/mongo
src/mongo/bson/bsonelement.cpp:165:                    if (strtol(e.fieldName(), 0, 10) > count) {
src/mongo/bson/oid_test.cpp:164:    ASSERT_EQUALS(term, std::stoi(oidTail));
src/mongo/scripting/mozjs/numberlong.cpp:172:            // For string values we call strtoll because we expect non-number string
src/mongo/util/net/cidr.cpp:64: * `std::stoi()` naturally throws `std::invalid_argument` if str
src/mongo/util/net/cidr.cpp:70:int strict_stoi(const std::string& str, int base = 10) {
src/mongo/util/net/cidr.cpp:72:    auto len = std::stoi(str, &pos, base);
src/mongo/util/net/cidr.cpp:123:    auto len = strict_stoi(std::string(slash + 1, end(s)), 10);
src/mongo/shell/shell_utils_launcher.cpp:319:                _port = strtol(str.c_str(), 0, 10);
src/mongo/platform/strtoll.h:35:static inline long long strtoll(const char* nptr, char** endptr, int base) {
src/mongo/platform/strtoll.h:36:    return _strtoi64(nptr, endptr, base);
src/mongo/dbtests/jsontests.cpp:767:        b.append("a", strtod("0.7", 0));
src/mongo/dbtests/jsontests.cpp:778:        b.append("a", strtod("-4.4433e-2", 0));
src/mongo/client/scram_client_cache.h:49: * intercepted SCRAM authentication data, or a stolen password
src/mongo/client/mongo_uri_connect.cpp:171:            socketTimeoutSecs = std::stod(it->second) / 1000;

kolisergej if you are interested in rebasing https://github.com/mongodb/mongo/pull/1128 and addressing the remaining instances, we could take another look at merging the PR. Before doing so we would need to make a decision about the std::sto family of functions.

Comment by Kolibaba Sergey [ 29/Dec/16 ]

Good day. Here is possible improvement

Comment by Andy Schwerin [ 13/Mar/13 ]

Not too many occurrences.

$ grep --exclude='json.*' --exclude='parse_number.*' --exclude="*vcx*" -IrEn '(strto|ato)[lid]' src/mongo
src/mongo/client/examples/httpClientTest.cpp:48:        port = atoi( argv[ 2 ] );
src/mongo/client/examples/rs.cpp:66:            nThreads = atoi( argv[++i] );
src/mongo/db/dbwebserver.cpp:430:                        b.append( "repl" , atoi( params["repl"].valuestr() ) );
src/mongo/db/jsobj.cpp:134:                    if (strtol(e.fieldName(), 0, 10) > count) {
src/mongo/db/jsobj.cpp:1328:            append( fieldName , atoi( data.c_str() ) );
src/mongo/db/repl/master_slave.cpp:698:            return set( atoi( str.c_str() ) );
src/mongo/db/restapi.cpp:144:                double number = strtod( val , &temp );
src/mongo/db/restapi.cpp:237:                return atoi( e.valuestr() );
src/mongo/dbtests/btreetests.inl:560:            unsigned long long num = strtol( spec + 1, &endPtr, 16 );
src/mongo/dbtests/btreetests.inl:563:                len = strtol( endPtr + 1, 0, 16 );
src/mongo/dbtests/jsontests.cpp:560:                b.append( "a", strtod( "0.7", 0 ) );
src/mongo/dbtests/jsontests.cpp:571:                b.append( "a", strtod( "-4.4433e-2", 0 ) );
src/mongo/platform/strtoll.h:22:static inline long long strtoll(const char* nptr, char** endptr, int base) {
src/mongo/platform/strtoll.h:23:    return _strtoi64(nptr, endptr, base);
src/mongo/s/strategy_single.cpp:196:                        int opid = atoi( s.substr( i + 1 ).c_str() );
src/mongo/scripting/engine_spidermonkey.cpp:533:                    verify( JS_SetElement( _context , array , atoi(e.fieldName()) , &v ) );
src/mongo/shell/shell_utils_launcher.cpp:222:                    _port = strtol( str.c_str(), 0, 10 );
src/mongo/tools/bridge.cpp:165:            port = strtol( argv[ ++i ], 0, 10 );
src/mongo/tools/bridge.cpp:171:            delay = strtol( argv[ ++i ], 0, 10 );
src/mongo/tools/sniffer.cpp:494:                serverPorts.insert( atoi( args[ i ] ) );
src/mongo/util/net/hostandport.h:172:            int port = atoi(colon+1);
src/mongo/util/net/httpclient.cpp:72:            port = atoi( t.c_str() );
src/mongo/util/net/miniwebserver.cpp:105:        long len = strtol( lengthLoc, 0, 10 );
src/mongo/util/processinfo_linux2.cpp:350:            return atoll(meminfo.c_str()) * 1024;   // convert from kB to bytes
src/mongo/util/text.cpp:138:        ret = strtoll( n, &endPtr, 10 );
src/mongo/util/text.cpp:151:        ret = _strtoi64( n, &endPtr, 10 );
src/mongo/util/version.cpp:182:            int bar = atoi( foo );

Generated at Thu Feb 08 03:18:55 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.