diff options
author | Tor Brede Vekterli <vekterli@oath.com> | 2018-07-25 14:24:21 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@oath.com> | 2018-07-25 14:24:21 +0000 |
commit | 12c8f3005e202b31a8e0ff3816ce9d714a269046 (patch) | |
tree | a5d9ba0eedc49df2cea2dbdfea677c4c37ed3775 /storage/src | |
parent | e3af3d215feb1e416b27b92bbf421dde281f3a09 (diff) |
Remove stringref::c_str()
The expected semantics of c_str() (a null-terminated string) cannot
be satisfied with a string reference, so remove the function entirely
to prevent people from using it in buggy ways.
Replaces c_str() with data() in places where it is presumed safe,
otherwise constructs temporary string instances. Certain callsites
have been de-stringref'd in favor of regular strings, in particular
where C APIs have been transitively called. The vast majority of
these were called with string parameters anyway, so should not
cause much extra allocation.
Diffstat (limited to 'storage/src')
13 files changed, 40 insertions, 39 deletions
diff --git a/storage/src/tests/bucketdb/bucketinfotest.cpp b/storage/src/tests/bucketdb/bucketinfotest.cpp index 3eb8d60befd..0298c50866c 100644 --- a/storage/src/tests/bucketdb/bucketinfotest.cpp +++ b/storage/src/tests/bucketdb/bucketinfotest.cpp @@ -51,14 +51,14 @@ getBucketInfo(std::string nodeList, std::string order) { { vespalib::StringTokenizer tokenizer(order, ","); for (uint32_t i = 0; i < tokenizer.size(); i++) { - ordering.push_back(atoi(tokenizer[i].c_str())); + ordering.push_back(atoi(tokenizer[i].data())); } } vespalib::StringTokenizer tokenizer(nodeList, ","); for (uint32_t i = 0; i < tokenizer.size(); i++) { info.addNode(BucketCopy(0, - atoi(tokenizer[i].c_str()), + atoi(tokenizer[i].data()), api::BucketInfo(1,1,1)), ordering); } diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp index 559afffc795..56f88b7f98f 100644 --- a/storage/src/tests/distributor/bucketdbupdatertest.cpp +++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp @@ -1800,7 +1800,7 @@ parseInputData(const std::string& data, for (uint32_t i = 0; i < tokenizer.size(); i++) { vespalib::StringTokenizer tok2(tokenizer[i], ":"); - uint16_t node = atoi(tok2[0].c_str()); + uint16_t node = atoi(tok2[0].data()); state.setNodeReplied(node); auto &pendingTransition = state.getPendingBucketSpaceDbTransition(makeBucketSpace()); @@ -1811,19 +1811,19 @@ parseInputData(const std::string& data, vespalib::StringTokenizer tok4(tok3[j], "/"); pendingTransition.addNodeInfo( - document::BucketId(16, atoi(tok4[0].c_str())), + document::BucketId(16, atoi(tok4[0].data())), BucketCopy( timestamp, node, api::BucketInfo( - atoi(tok4[1].c_str()), - atoi(tok4[2].c_str()), - atoi(tok4[3].c_str()), - atoi(tok4[2].c_str()), - atoi(tok4[3].c_str())))); + atoi(tok4[1].data()), + atoi(tok4[2].data()), + atoi(tok4[3].data()), + atoi(tok4[2].data()), + atoi(tok4[3].data())))); } else { pendingTransition.addNodeInfo( - document::BucketId(16, atoi(tok3[j].c_str())), + document::BucketId(16, atoi(tok3[j].data())), BucketCopy(timestamp, node, api::BucketInfo(3, 3, 3, 3, 3))); diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp index ce20546dd44..46c756001d9 100644 --- a/storage/src/tests/distributor/distributortest.cpp +++ b/storage/src/tests/distributor/distributortest.cpp @@ -176,11 +176,11 @@ private: trusted = true; } - uint16_t node = atoi(tokenizer2[0].c_str()); + uint16_t node = atoi(tokenizer2[0].data()); if (tokenizer2[1] == "r") { removedNodes.push_back(node); } else { - uint32_t checksum = atoi(tokenizer2[1].c_str()); + uint32_t checksum = atoi(tokenizer2[1].data()); changedNodes.push_back( BucketCopy( i + 1, diff --git a/storage/src/tests/distributor/distributortestutil.cpp b/storage/src/tests/distributor/distributortestutil.cpp index e43161946fb..d3496d0c9f6 100644 --- a/storage/src/tests/distributor/distributortestutil.cpp +++ b/storage/src/tests/distributor/distributortestutil.cpp @@ -187,16 +187,16 @@ void DistributorTestUtil::addNodesToBucketDB(const document::Bucket& bucket, con vespalib::StringTokenizer tok2(tokenizer[i], "="); vespalib::StringTokenizer tok3(tok2[1], "/"); - api::BucketInfo info(atoi(tok3[0].c_str()), - atoi(tok3.size() > 1 ? tok3[1].c_str() : tok3[0].c_str()), - atoi(tok3.size() > 2 ? tok3[2].c_str() : tok3[0].c_str())); + api::BucketInfo info(atoi(tok3[0].data()), + atoi(tok3.size() > 1 ? tok3[1].data() : tok3[0].data()), + atoi(tok3.size() > 2 ? tok3[2].data() : tok3[0].data())); size_t flagsIdx = 3; // Meta info override? For simplicity, require both meta count and size if (tok3.size() > 4 && (!tok3[3].empty() && isdigit(tok3[3][0]))) { - info.setMetaCount(atoi(tok3[3].c_str())); - info.setUsedFileSize(atoi(tok3[4].c_str())); + info.setMetaCount(atoi(tok3[3].data())); + info.setUsedFileSize(atoi(tok3[4].data())); flagsIdx = 5; } @@ -211,7 +211,7 @@ void DistributorTestUtil::addNodesToBucketDB(const document::Bucket& bucket, con info.setReady(false); } - uint16_t idx = atoi(tok2[0].c_str()); + uint16_t idx = atoi(tok2[0].data()); BucketCopy node( 0, idx, diff --git a/storage/src/tests/distributor/putoperationtest.cpp b/storage/src/tests/distributor/putoperationtest.cpp index b43d3cf64ad..5551d0a5010 100644 --- a/storage/src/tests/distributor/putoperationtest.cpp +++ b/storage/src/tests/distributor/putoperationtest.cpp @@ -572,8 +572,8 @@ parseBucketInfoString(const std::string& nodeList) { BucketInfo entry; for (uint32_t i = 0; i < tokenizer.size(); i++) { vespalib::StringTokenizer tokenizer2(tokenizer[i], "-"); - int node = atoi(tokenizer2[0].c_str()); - int size = atoi(tokenizer2[1].c_str()); + int node = atoi(tokenizer2[0].data()); + int size = atoi(tokenizer2[1].data()); bool trusted = (tokenizer2[2] == "true"); entry.addNode(BucketCopy(0, diff --git a/storage/src/tests/storageserver/fnet_listener_test.cpp b/storage/src/tests/storageserver/fnet_listener_test.cpp index cc9c424ac28..84051041d25 100644 --- a/storage/src/tests/storageserver/fnet_listener_test.cpp +++ b/storage/src/tests/storageserver/fnet_listener_test.cpp @@ -135,7 +135,7 @@ vespalib::string make_compressable_state_string() { ss << " ." << i << ".s:d"; } return vespalib::make_string("version:123 distributor:100%s storage:100%s", - ss.str().c_str(), ss.str().c_str()); + ss.str().data(), ss.str().data()); } } diff --git a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp b/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp index 0ff178c3f0f..c8d73737d7c 100644 --- a/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp +++ b/storage/src/vespa/storage/common/hostreporter/kernelmetrictool.cpp @@ -65,8 +65,9 @@ uint32_t getTokenCount(const vespalib::string& line) { uint64_t toLong(const vespalib::stringref& s, int base) { char* endptr; - uint64_t result(strtoull(s.c_str(), &endptr, base)); - if ((s.c_str() + s.size()) != endptr) { + // FIXME C++17 range-safe from_chars() instead of strtoull() + uint64_t result(strtoull(s.data(), &endptr, base)); + if ((s.data() + s.size()) != endptr) { throw vespalib::IllegalArgumentException("Parsing '" + s + "' as a long."); } return result; diff --git a/storage/src/vespa/storage/frameworkimpl/thread/deadlockdetector.cpp b/storage/src/vespa/storage/frameworkimpl/thread/deadlockdetector.cpp index 7c6a708fd8b..e1b7d9c52ea 100644 --- a/storage/src/vespa/storage/frameworkimpl/thread/deadlockdetector.cpp +++ b/storage/src/vespa/storage/frameworkimpl/thread/deadlockdetector.cpp @@ -169,7 +169,7 @@ namespace { } else if (state != DeadLockDetector::OK) { vespalib::asciistream ost; ost << "Thread " << id << " has registered tick again.\n"; - LOGBP(info, "%s", ost.str().c_str()); + LOGBP(info, "%s", ost.str().data()); state = DeadLockDetector::OK; } } @@ -200,7 +200,7 @@ DeadLockDetector::handleDeadlock(const framework::MilliSecTime& currentTime, if (warnOnly) { if (_enableWarning) { LOGBT(warning, "deadlockw-" + id, "%s", - error.str().c_str()); + error.str().data()); if (_reportedBucketDBLocksAtState != WARNED) { _reportedBucketDBLocksAtState = WARNED; LOG(info, "Locks in bucket database at deadlock time:" @@ -212,7 +212,7 @@ DeadLockDetector::handleDeadlock(const framework::MilliSecTime& currentTime, } else { if (_enableShutdown || _enableWarning) { LOGBT(error, "deadlock-" + id, "%s", - error.str().c_str()); + error.str().data()); } } if (!_enableShutdown) return; diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index 9d4d7223411..961af1532a1 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -432,7 +432,7 @@ FileStorManager::onDeleteBucket(const shared_ptr<api::DeleteBucketCommand>& cmd) << ", but storage bucket database contains " << entry->getBucketInfo().toString(); - LOG(debug, "Rejecting bucket delete: %s", ost.str().c_str()); + LOG(debug, "Rejecting bucket delete: %s", ost.str().data()); std::shared_ptr<api::StorageReply> reply = cmd->makeReply(); static_cast<api::DeleteBucketReply&>(*reply).setBucketInfo(entry->getBucketInfo()); reply->setResult(api::ReturnCode(api::ReturnCode::REJECTED, ost.str())); diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index 370f1c85241..22af1a73633 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -787,7 +787,7 @@ MergeThrottler::validateNewMerge( oss << mergeCmd.toString() << " sent to node " << _component.getIndex() << ", which is not in its forwarding chain"; - LOG(error, "%s", oss.str().c_str()); + LOG(error, "%s", oss.str().data()); } else if (mergeCmd.getChain().size() >= nodeSeq.getSortedNodes().size()) { // Chain is full but we haven't seen the merge! This means // the node has probably gone down with a merge it previously @@ -795,12 +795,12 @@ MergeThrottler::validateNewMerge( oss << mergeCmd.toString() << " is not in node's internal state, but has a " << "full chain, meaning it cannot be forwarded."; - LOG(debug, "%s", oss.str().c_str()); + LOG(debug, "%s", oss.str().data()); } else if (nodeSeq.chainContainsIndex(nodeSeq.getThisNodeIndex())) { oss << mergeCmd.toString() << " is not in node's internal state, but contains " << "this node in its non-full chain. This should not happen!"; - LOG(error, "%s", oss.str().c_str()); + LOG(error, "%s", oss.str().data()); } else { valid = true; } @@ -1117,7 +1117,7 @@ MergeThrottler::makeAbortReply(api::StorageCommand& cmd, vespalib::stringref reason) const { LOG(debug, "Aborting message %s with reason '%s'", - cmd.toString().c_str(), reason.c_str()); + cmd.toString().c_str(), reason.data()); std::unique_ptr<api::StorageReply> reply(cmd.makeReply()); reply->setResult(api::ReturnCode(api::ReturnCode::ABORTED, reason)); return std::shared_ptr<api::StorageMessage>(reply.release()); diff --git a/storage/src/vespa/storage/storageserver/service_layer_error_listener.cpp b/storage/src/vespa/storage/storageserver/service_layer_error_listener.cpp index 41177fe46b8..e26549a3b37 100644 --- a/storage/src/vespa/storage/storageserver/service_layer_error_listener.cpp +++ b/storage/src/vespa/storage/storageserver/service_layer_error_listener.cpp @@ -15,21 +15,21 @@ void ServiceLayerErrorListener::on_fatal_error(vespalib::stringref message) { LOG(info, "Received FATAL_ERROR from persistence provider, " "shutting down node: %s", - message.c_str()); + vespalib::string(message).c_str()); _component.requestShutdown(message); // Thread safe } else { LOG(debug, "Received FATAL_ERROR from persistence provider: %s. " "Node has already been instructed to shut down so " "not doing anything now.", - message.c_str()); + vespalib::string(message).c_str()); } } void ServiceLayerErrorListener::on_resource_exhaustion_error(vespalib::stringref message) { LOG(debug, "SPI reports resource exhaustion ('%s'). " "Applying back-pressure to merge throttler", - message.c_str()); + vespalib::string(message).c_str()); _merge_throttler.apply_timed_backpressure(); // Thread safe } diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp index d6ca8c266d3..77d9299169f 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.cpp +++ b/storage/src/vespa/storage/storageserver/statemanager.cpp @@ -375,7 +375,7 @@ considerInsertDerivedTransition(const lib::State ¤tBaseline, ((currentDerived != currentBaseline) || (newDerived != newBaseline))); if (considerDerivedTransition && (transitions.find(bucketSpace) == transitions.end())) { transitions[bucketSpace] = vespalib::make_string("%s space: '%s' to '%s'", - document::FixedBucketSpaces::to_string(bucketSpace).c_str(), + document::FixedBucketSpaces::to_string(bucketSpace).data(), currentDerived.getName().c_str(), newDerived.getName().c_str()); } diff --git a/storage/src/vespa/storage/visiting/visitorthread.cpp b/storage/src/vespa/storage/visiting/visitorthread.cpp index b12a1eb6e7f..6bf28b08540 100644 --- a/storage/src/vespa/storage/visiting/visitorthread.cpp +++ b/storage/src/vespa/storage/visiting/visitorthread.cpp @@ -253,7 +253,7 @@ VisitorThread::run(framework::ThreadHandle& thread) } catch (std::exception& e) { vespalib::asciistream ost; ost << "Failed to handle visitor message:" << e.what(); - LOG(warning, "Failed handling visitor message: %s", ost.str().c_str()); + LOG(warning, "Failed handling visitor message: %s", ost.str().data()); result = ReturnCode(ReturnCode::INTERNAL_FAILURE, ost.str()); if (entry._message.get() && entry._message->getType() == api::MessageType::VISITOR_CREATE) { _messageSender.closed(entry._visitorId); @@ -466,7 +466,7 @@ VisitorThread::onCreateVisitor( if (visitor.get() == 0) { result = ReturnCode(ReturnCode::ILLEGAL_PARAMETERS, errors.str()); LOG(warning, "CreateVisitor(%s): Failed to create visitor: %s", - cmd->getInstanceId().c_str(), errors.str().c_str()); + cmd->getInstanceId().c_str(), errors.str().data()); break; } // Set visitor parameters @@ -510,7 +510,7 @@ VisitorThread::onCreateVisitor( << cmd->getDocumentSelection() << "': " << e.getMessage(); result = ReturnCode(ReturnCode::ILLEGAL_PARAMETERS, ost.str()); LOG(warning, "CreateVisitor(%s): %s", - cmd->getInstanceId().c_str(), ost.str().c_str()); + cmd->getInstanceId().c_str(), ost.str().data()); break; } catch (document::select::ParsingFailedException& e) { vespalib::asciistream ost; @@ -518,7 +518,7 @@ VisitorThread::onCreateVisitor( << cmd->getDocumentSelection() << "': " << e.getMessage(); result = ReturnCode(ReturnCode::ILLEGAL_PARAMETERS, ost.str()); LOG(warning, "CreateVisitor(%s): %s", - cmd->getInstanceId().c_str(), ost.str().c_str()); + cmd->getInstanceId().c_str(), ost.str().data()); break; } LOG(debug, "CreateVisitor(%s): Successfully created visitor", |