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.

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 &currentBaseline,
             ((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",