From 8bc5a10d6d0295e60da62a754e435e0dd6ae536f Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Thu, 31 Oct 2019 16:02:40 +0100 Subject: [PATCH] Further const correctness / String by reference passing cleanups (#6571) There are actually several instances where we pass in read-only parameters as pass-by-value, where in the case of String() that is inefficient as it involves copy-constructor/temp string creations. We can avoid that, similarly to single character string concatenations done via string literals instead of char literals. --- libraries/ArduinoOTA/ArduinoOTA.cpp | 2 +- .../src/ESP8266HTTPClient.cpp | 55 ++++++++++--------- .../ESP8266HTTPClient/src/ESP8266HTTPClient.h | 26 ++++----- .../src/ESP8266WebServer-impl.h | 14 ++--- .../ESP8266WebServer/src/ESP8266WebServer.h | 6 +- .../ESP8266WiFiMesh/src/ESP8266WiFiMesh.cpp | 8 +-- .../src/ESP8266httpUpdate.cpp | 2 +- .../ESP8266httpUpdate/src/ESP8266httpUpdate.h | 2 +- .../ESP8266mDNS/src/ESP8266mDNS_Legacy.cpp | 2 +- libraries/ESP8266mDNS/src/LEAmDNS.cpp | 18 +++--- libraries/ESP8266mDNS/src/LEAmDNS.h | 18 +++--- 11 files changed, 78 insertions(+), 75 deletions(-) diff --git a/libraries/ArduinoOTA/ArduinoOTA.cpp b/libraries/ArduinoOTA/ArduinoOTA.cpp index 53d5272d8..0bc92989a 100644 --- a/libraries/ArduinoOTA/ArduinoOTA.cpp +++ b/libraries/ArduinoOTA/ArduinoOTA.cpp @@ -231,7 +231,7 @@ void ArduinoOTAClass::_onRx(){ return; } - String challenge = _password + ":" + String(_nonce) + ":" + cnonce; + String challenge = _password + ':' + String(_nonce) + ':' + cnonce; MD5Builder _challengemd5; _challengemd5.begin(); _challengemd5.add(challenge); diff --git a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp index a05b9dd6f..9c7b4d0ce 100644 --- a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp +++ b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp @@ -150,7 +150,7 @@ void HTTPClient::clear() * @param https bool * @return success bool */ -bool HTTPClient::begin(WiFiClient &client, String url) { +bool HTTPClient::begin(WiFiClient &client, const String& url) { #if HTTPCLIENT_1_1_COMPATIBLE if(_tcpDeprecated) { DEBUG_HTTPCLIENT("[HTTP-Client][begin] mix up of new and deprecated api\n"); @@ -188,7 +188,7 @@ bool HTTPClient::begin(WiFiClient &client, String url) { * @param https bool * @return success bool */ -bool HTTPClient::begin(WiFiClient &client, String host, uint16_t port, String uri, bool https) +bool HTTPClient::begin(WiFiClient &client, const String& host, uint16_t port, const String& uri, bool https) { #if HTTPCLIENT_1_1_COMPATIBLE if(_tcpDeprecated) { @@ -281,8 +281,10 @@ bool HTTPClient::begin(String url) } #endif // HTTPCLIENT_1_1_COMPATIBLE -bool HTTPClient::beginInternal(String url, const char* expectedProtocol) +bool HTTPClient::beginInternal(const String& __url, const char* expectedProtocol) { + String url(__url); + DEBUG_HTTPCLIENT("[HTTP-Client][begin] url: %s\n", url.c_str()); clear(); @@ -500,7 +502,7 @@ void HTTPClient::setAuthorization(const char * user, const char * password) { if(user && password) { String auth = user; - auth += ":"; + auth += ':'; auth += password; _base64Authorization = base64::encode(auth); } @@ -533,7 +535,7 @@ void HTTPClient::setTimeout(uint16_t timeout) * set the URL to a new value. Handy for following redirects. * @param url */ -bool HTTPClient::setURL(String url) +bool HTTPClient::setURL(const String& url) { // if the new location is only a path then only update the URI if (url && url[0] == '/') { @@ -542,7 +544,7 @@ bool HTTPClient::setURL(String url) return true; } - if (!url.startsWith(_protocol + ":")) { + if (!url.startsWith(_protocol + ':')) { DEBUG_HTTPCLIENT("[HTTP-Client][setURL] new URL not the same protocol, expected '%s', URL: '%s'\n", _protocol.c_str(), url.c_str()); return false; } @@ -587,16 +589,16 @@ int HTTPClient::GET() /** * sends a post request to the server - * @param payload uint8_t * + * @param payload const uint8_t * * @param size size_t * @return http code */ -int HTTPClient::POST(uint8_t * payload, size_t size) +int HTTPClient::POST(const uint8_t* payload, size_t size) { return sendRequest("POST", payload, size); } -int HTTPClient::POST(String payload) +int HTTPClient::POST(const String& payload) { return POST((uint8_t *) payload.c_str(), payload.length()); } @@ -607,26 +609,26 @@ int HTTPClient::POST(String payload) * @param size size_t * @return http code */ -int HTTPClient::PUT(uint8_t * payload, size_t size) { +int HTTPClient::PUT(const uint8_t* payload, size_t size) { return sendRequest("PUT", payload, size); } -int HTTPClient::PUT(String payload) { - return PUT((uint8_t *) payload.c_str(), payload.length()); +int HTTPClient::PUT(const String& payload) { + return PUT((const uint8_t *) payload.c_str(), payload.length()); } /** * sends a patch request to the server - * @param payload uint8_t * + * @param payload const uint8_t * * @param size size_t * @return http code */ -int HTTPClient::PATCH(uint8_t * payload, size_t size) { +int HTTPClient::PATCH(const uint8_t * payload, size_t size) { return sendRequest("PATCH", payload, size); } -int HTTPClient::PATCH(String payload) { - return PATCH((uint8_t *) payload.c_str(), payload.length()); +int HTTPClient::PATCH(const String& payload) { + return PATCH((const uint8_t *) payload.c_str(), payload.length()); } /** @@ -635,19 +637,19 @@ int HTTPClient::PATCH(String payload) { * @param payload String data for the message body * @return */ -int HTTPClient::sendRequest(const char * type, String payload) +int HTTPClient::sendRequest(const char * type, const String& payload) { - return sendRequest(type, (uint8_t *) payload.c_str(), payload.length()); + return sendRequest(type, (const uint8_t *) payload.c_str(), payload.length()); } /** * sendRequest - * @param type const char * "GET", "POST", .... - * @param payload uint8_t * data for the message body if null not send - * @param size size_t size for the message body if 0 not send + * @param type const char * "GET", "POST", .... + * @param payload const uint8_t * data for the message body if null not send + * @param size size_t size for the message body if 0 not send * @return -1 if no info or > 0 when Content-Length is set by server */ -int HTTPClient::sendRequest(const char * type, uint8_t * payload, size_t size) +int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t size) { bool redirect = false; int code = 0; @@ -1212,12 +1214,12 @@ bool HTTPClient::sendHeader(const char * type) return false; } - String header = String(type) + " " + (_uri.length() ? _uri : F("/")) + F(" HTTP/1."); + String header = String(type) + ' ' + (_uri.length() ? _uri : F("/")) + F(" HTTP/1."); if(_useHTTP10) { - header += "0"; + header += '0'; } else { - header += "1"; + header += '1'; } header += String(F("\r\nHost: ")) + _host; @@ -1316,7 +1318,8 @@ int HTTPClient::handleHeaderResponse() if(_currentHeaders[i].key.equalsIgnoreCase(headerName)) { if (_currentHeaders[i].value != "") { // Existing value, append this one with a comma - _currentHeaders[i].value += "," + headerValue; + _currentHeaders[i].value += ','; + _currentHeaders[i].value += headerValue; } else { _currentHeaders[i].value = headerValue; } diff --git a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h index d6e4f88c0..1d7548f0f 100644 --- a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h +++ b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h @@ -147,8 +147,8 @@ public: * Since both begin() functions take a reference to client as a parameter, you need to * ensure the client object lives the entire time of the HTTPClient */ - bool begin(WiFiClient &client, String url); - bool begin(WiFiClient &client, String host, uint16_t port, String uri = "/", bool https = false); + bool begin(WiFiClient &client, const String& url); + bool begin(WiFiClient &client, const String& host, uint16_t port, const String& uri = "/", bool https = false); #if HTTPCLIENT_1_1_COMPATIBLE // Plain HTTP connection, unencrypted @@ -175,20 +175,20 @@ public: void setTimeout(uint16_t timeout); void setFollowRedirects(bool follow); void setRedirectLimit(uint16_t limit); // max redirects to follow for a single request - bool setURL(String url); // handy for handling redirects + bool setURL(const String& url); // handy for handling redirects void useHTTP10(bool usehttp10 = true); /// request handling int GET(); - int POST(uint8_t * payload, size_t size); - int POST(String payload); - int PUT(uint8_t * payload, size_t size); - int PUT(String payload); - int PATCH(uint8_t * payload, size_t size); - int PATCH(String payload); - int sendRequest(const char * type, String payload); - int sendRequest(const char * type, uint8_t * payload = NULL, size_t size = 0); - int sendRequest(const char * type, Stream * stream, size_t size = 0); + int POST(const uint8_t* payload, size_t size); + int POST(const String& payload); + int PUT(const uint8_t* payload, size_t size); + int PUT(const String& payload); + int PATCH(const uint8_t* payload, size_t size); + int PATCH(const String& payload); + int sendRequest(const char* type, const String& payload); + int sendRequest(const char* type, const uint8_t* payload = NULL, size_t size = 0); + int sendRequest(const char* type, Stream * stream, size_t size = 0); void addHeader(const String& name, const String& value, bool first = false, bool replace = true); @@ -216,7 +216,7 @@ protected: String value; }; - bool beginInternal(String url, const char* expectedProtocol); + bool beginInternal(const String& url, const char* expectedProtocol); void disconnect(bool preserveClient = false); void clear(); int returnError(int error); diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h index 3af7ca394..1efcf92e5 100644 --- a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h +++ b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h @@ -252,11 +252,11 @@ void ESP8266WebServerTemplate::requestAuthentication(HTTPAuthMethod _srealm = String(realm); } if(mode == BASIC_AUTH) { - sendHeader(String(FPSTR(WWW_Authenticate)), String(F("Basic realm=\"")) + _srealm + String(F("\""))); + sendHeader(String(FPSTR(WWW_Authenticate)), String(F("Basic realm=\"")) + _srealm + String('\"')); } else { _snonce=_getRandomHexString(); _sopaque=_getRandomHexString(); - sendHeader(String(FPSTR(WWW_Authenticate)), String(F("Digest realm=\"")) +_srealm + String(F("\", qop=\"auth\", nonce=\"")) + _snonce + String(F("\", opaque=\"")) + _sopaque + String(F("\""))); + sendHeader(String(FPSTR(WWW_Authenticate)), String(F("Digest realm=\"")) +_srealm + String(F("\", qop=\"auth\", nonce=\"")) + _snonce + String(F("\", opaque=\"")) + _sopaque + String('\"')); } using namespace mime; send(401, String(FPSTR(mimeTable[html].mimeType)), authFailMsg); @@ -524,13 +524,13 @@ String ESP8266WebServerTemplate::credentialHash(const String& userna { MD5Builder md5; md5.begin(); - md5.add(username + ":" + realm + ":" + password); // md5 of the user:realm:password + md5.add(username + ':' + realm + ':' + password); // md5 of the user:realm:password md5.calculate(); return md5.toString(); } template -void ESP8266WebServerTemplate::_streamFileCore(const size_t fileSize, const String & fileName, const String & contentType) +void ESP8266WebServerTemplate::_streamFileCore(const size_t fileSize, const String &fileName, const String &contentType) { using namespace mime; setContentLength(fileSize); @@ -544,7 +544,7 @@ void ESP8266WebServerTemplate::_streamFileCore(const size_t fileSize template -const String& ESP8266WebServerTemplate::arg(String name) const { +const String& ESP8266WebServerTemplate::arg(const String& name) const { for (int j = 0; j < _postArgsLen; ++j) { if ( _postArgs[j].key == name ) return _postArgs[j].value; @@ -590,7 +590,7 @@ bool ESP8266WebServerTemplate::hasArg(const String& name) const { template -const String& ESP8266WebServerTemplate::header(String name) const { +const String& ESP8266WebServerTemplate::header(const String& name) const { for (int i = 0; i < _headerKeysCount; ++i) { if (_currentHeaders[i].key.equalsIgnoreCase(name)) return _currentHeaders[i].value; @@ -630,7 +630,7 @@ int ESP8266WebServerTemplate::headers() const { } template -bool ESP8266WebServerTemplate::hasHeader(String name) const { +bool ESP8266WebServerTemplate::hasHeader(const String& name) const { for (int i = 0; i < _headerKeysCount; ++i) { if ((_currentHeaders[i].key.equalsIgnoreCase(name)) && (_currentHeaders[i].value.length() > 0)) return true; diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer.h b/libraries/ESP8266WebServer/src/ESP8266WebServer.h index 9b7c2bbb3..4281ce863 100644 --- a/libraries/ESP8266WebServer/src/ESP8266WebServer.h +++ b/libraries/ESP8266WebServer/src/ESP8266WebServer.h @@ -107,17 +107,17 @@ public: // Allows setting server options (i.e. SSL keys) by the instantiator ServerType &getServer() { return _server; } - const String& arg(String name) const; // get request argument value by name + const String& arg(const String& name) const; // get request argument value by name const String& arg(int i) const; // get request argument value by number const String& argName(int i) const; // get request argument name by number int args() const; // get arguments count bool hasArg(const String& name) const; // check if argument exists void collectHeaders(const char* headerKeys[], const size_t headerKeysCount); // set the request headers to collect - const String& header(String name) const; // get request header value by name + const String& header(const String& name) const; // get request header value by name const String& header(int i) const; // get request header value by number const String& headerName(int i) const; // get request header name by number int headers() const; // get header count - bool hasHeader(String name) const; // check if header exists + bool hasHeader(const String& name) const; // check if header exists const String& hostHeader() const; // get request host header if available or empty String if not // send response to the client diff --git a/libraries/ESP8266WiFiMesh/src/ESP8266WiFiMesh.cpp b/libraries/ESP8266WiFiMesh/src/ESP8266WiFiMesh.cpp index fead562e6..c3fb4774d 100644 --- a/libraries/ESP8266WiFiMesh/src/ESP8266WiFiMesh.cpp +++ b/libraries/ESP8266WiFiMesh/src/ESP8266WiFiMesh.cpp @@ -358,7 +358,7 @@ transmission_status_t ESP8266WiFiMesh::exchangeInfo(WiFiClient &currClient) { verboseModePrint("Transmitting"); // Not storing strings in flash (via F()) to avoid performance impacts when using the string. - currClient.print(getMessage() + "\r"); + currClient.print(getMessage() + '\r'); yield(); if (!waitForClientTransmission(currClient, _stationModeTimeoutMs)) @@ -582,11 +582,11 @@ void ESP8266WiFiMesh::attemptTransmission(const String &message, bool concluding if(_verboseMode) // Avoid string generation if not required { - verboseModePrint(String(F("AP acquired: ")) + currentSSID + String(F(", Ch:")) + String(currentWiFiChannel) + " ", false); + verboseModePrint(String(F("AP acquired: ")) + currentSSID + String(F(", Ch:")) + String(currentWiFiChannel) + ' ', false); if(currentNetwork.networkIndex != NETWORK_INFO_DEFAULT_INT) { - verboseModePrint("(" + String(WiFi.RSSI(currentNetwork.networkIndex)) + String(F("dBm) ")) + + verboseModePrint(String('(') + String(WiFi.RSSI(currentNetwork.networkIndex)) + String(F("dBm) ")) + (WiFi.encryptionType(currentNetwork.networkIndex) == ENC_TYPE_NONE ? String(F("open")) : ""), false); } @@ -662,7 +662,7 @@ void ESP8266WiFiMesh::acceptRequest() if (_client.connected()) { verboseModePrint("Responding"); // Not storing strings in flash (via F()) to avoid performance impacts when using the string. - _client.print(response + "\r"); + _client.print(response + '\r'); _client.flush(); yield(); } diff --git a/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp b/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp index 8787562d6..30ccf2dbf 100644 --- a/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp +++ b/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp @@ -439,7 +439,7 @@ HTTPUpdateResult ESP8266HTTPUpdate::handleUpdate(HTTPClient& http, const String& * @param md5 String * @return true if Update ok */ -bool ESP8266HTTPUpdate::runUpdate(Stream& in, uint32_t size, String md5, int command) +bool ESP8266HTTPUpdate::runUpdate(Stream& in, uint32_t size, const String& md5, int command) { StreamString error; diff --git a/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.h b/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.h index 14c0b6552..93d68f811 100644 --- a/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.h +++ b/libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.h @@ -130,7 +130,7 @@ public: protected: t_httpUpdate_return handleUpdate(HTTPClient& http, const String& currentVersion, bool spiffs = false); - bool runUpdate(Stream& in, uint32_t size, String md5, int command = U_FLASH); + bool runUpdate(Stream& in, uint32_t size, const String& md5, int command = U_FLASH); int _lastError; bool _rebootOnUpdate = true; diff --git a/libraries/ESP8266mDNS/src/ESP8266mDNS_Legacy.cpp b/libraries/ESP8266mDNS/src/ESP8266mDNS_Legacy.cpp index af98b46b6..879119552 100644 --- a/libraries/ESP8266mDNS/src/ESP8266mDNS_Legacy.cpp +++ b/libraries/ESP8266mDNS/src/ESP8266mDNS_Legacy.cpp @@ -277,7 +277,7 @@ bool MDNSResponder::addServiceTxt(char *name, char *proto, char *key, char *valu return false; //max txt record size } MDNSTxt *newtxt = new MDNSTxt; - newtxt->_txt = String(key) + "=" + String(value); + newtxt->_txt = String(key) + '=' + String(value); newtxt->_next = 0; if (servicePtr->_txts == 0) //no services have been added { diff --git a/libraries/ESP8266mDNS/src/LEAmDNS.cpp b/libraries/ESP8266mDNS/src/LEAmDNS.cpp index 95f07079d..b76ce56e4 100644 --- a/libraries/ESP8266mDNS/src/LEAmDNS.cpp +++ b/libraries/ESP8266mDNS/src/LEAmDNS.cpp @@ -280,7 +280,7 @@ bool MDNSResponder::setHostname(const char* p_pcHostname) /* MDNSResponder::setHostname (LEGACY) */ -bool MDNSResponder::setHostname(String p_strHostname) +bool MDNSResponder::setHostname(const String& p_strHostname) { return setHostname(p_strHostname.c_str()); @@ -369,8 +369,8 @@ bool MDNSResponder::removeService(const char* p_pcName, /* MDNSResponder::addService (LEGACY) */ -bool MDNSResponder::addService(String p_strService, - String p_strProtocol, +bool MDNSResponder::addService(const String& p_strService, + const String& p_strProtocol, uint16_t p_u16Port) { @@ -595,10 +595,10 @@ bool MDNSResponder::addServiceTxt(const char* p_pcService, /* MDNSResponder::addServiceTxt (LEGACY) */ -bool MDNSResponder::addServiceTxt(String p_strService, - String p_strProtocol, - String p_strKey, - String p_strValue) +bool MDNSResponder::addServiceTxt(const String& p_strService, + const String& p_strProtocol, + const String& p_strKey, + const String& p_strValue) { return (0 != _addServiceTxt(_findService(m_pcHostname, p_strService.c_str(), p_strProtocol.c_str()), p_strKey.c_str(), p_strValue.c_str(), false)); @@ -826,8 +826,8 @@ bool MDNSResponder::removeQuery(void) /* MDNSResponder::queryService (LEGACY) */ -uint32_t MDNSResponder::queryService(String p_strService, - String p_strProtocol) +uint32_t MDNSResponder::queryService(const String& p_strService, + const String& p_strProtocol) { return queryService(p_strService.c_str(), p_strProtocol.c_str()); diff --git a/libraries/ESP8266mDNS/src/LEAmDNS.h b/libraries/ESP8266mDNS/src/LEAmDNS.h index da3aa01a1..39b2bad81 100644 --- a/libraries/ESP8266mDNS/src/LEAmDNS.h +++ b/libraries/ESP8266mDNS/src/LEAmDNS.h @@ -192,7 +192,7 @@ public: // Change hostname (probing is restarted) bool setHostname(const char* p_pcHostname); // for compatibility... - bool setHostname(String p_strHostname); + bool setHostname(const String& p_strHostname); /** hMDNSService (opaque handle to access the service) @@ -213,8 +213,8 @@ public: const char* p_pcServiceName, const char* p_pcProtocol); // for compatibility... - bool addService(String p_strServiceName, - String p_strProtocol, + bool addService(const String& p_strServiceName, + const String& p_strProtocol, uint16_t p_u16Port); @@ -276,10 +276,10 @@ public: const char* p_pcProtocol, const char* p_pcKey, const char* p_pcValue); - bool addServiceTxt(String p_strService, - String p_strProtocol, - String p_strKey, - String p_strValue); + bool addServiceTxt(const String& p_strService, + const String& p_strProtocol, + const String& p_strKey, + const String& p_strValue); /** MDNSDynamicServiceTxtCallbackFn @@ -331,8 +331,8 @@ public: const uint16_t p_u16Timeout = MDNS_QUERYSERVICES_WAIT_TIME); bool removeQuery(void); // for compatibility... - uint32_t queryService(String p_strService, - String p_strProtocol); + uint32_t queryService(const String& p_strService, + const String& p_strProtocol); const char* answerHostname(const uint32_t p_u32AnswerIndex); IPAddress answerIP(const uint32_t p_u32AnswerIndex);