From 2050bab09c7b3917caa92133299da0f7c07bd38d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 5 Dec 2017 13:23:31 +0100 Subject: [PATCH] src: minor cleanups to node_url.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove pointless pointers - Make `WriteHost` take a const argument so that it’s functionality is clear from the signature - Make `FindLongestZeroSequence` templated to accommodate the constness in `WriteHost` and because using `uint16_t` is an articifial, unnecessary restriction - Remove string copying when no copies are needed - Make `PercentDecode` just return its return value - Make `ParseHost` (string-only version) take its constant argument as a constant reference PR-URL: https://github.com/nodejs/node/pull/17470 Reviewed-By: Timothy Gu --- src/node_url.cc | 61 +++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/src/node_url.cc b/src/node_url.cc index e1ef9273ae927e..b4b16399aab87f 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -506,12 +506,11 @@ static inline unsigned hex2bin(const T ch) { return static_cast(-1); } -static inline void PercentDecode(const char* input, - size_t len, - std::string* dest) { +inline std::string PercentDecode(const char* input, size_t len) { + std::string dest; if (len == 0) - return; - dest->reserve(len); + return dest; + dest.reserve(len); const char* pointer = input; const char* end = input + len; @@ -522,17 +521,18 @@ static inline void PercentDecode(const char* input, (ch == '%' && (!IsASCIIHexDigit(pointer[1]) || !IsASCIIHexDigit(pointer[2])))) { - *dest += ch; + dest += ch; pointer++; continue; } else { unsigned a = hex2bin(pointer[1]); unsigned b = hex2bin(pointer[2]); char c = static_cast(a * 16 + b); - *dest += c; + dest += c; pointer += 3; } } + return dest; } #define SPECIALS(XX) \ @@ -860,7 +860,7 @@ static url_host_type ParseHost(url_host* host, return ParseOpaqueHost(host, input, length); // First, we have to percent decode - PercentDecode(input, length, &decoded); + decoded = PercentDecode(input, length); // Then we have to punycode toASCII if (!ToASCII(decoded, &decoded)) @@ -894,13 +894,13 @@ static url_host_type ParseHost(url_host* host, // Locates the longest sequence of 0 segments in an IPv6 address // in order to use the :: compression when serializing -static inline uint16_t* FindLongestZeroSequence(uint16_t* values, - size_t len) { - uint16_t* start = values; - uint16_t* end = start + len; - uint16_t* result = nullptr; +template +static inline T* FindLongestZeroSequence(T* values, size_t len) { + T* start = values; + T* end = start + len; + T* result = nullptr; - uint16_t* current = nullptr; + T* current = nullptr; unsigned counter = 0, longest = 1; while (start < end) { @@ -923,7 +923,7 @@ static inline uint16_t* FindLongestZeroSequence(uint16_t* values, return result; } -static url_host_type WriteHost(url_host* host, std::string* dest) { +static url_host_type WriteHost(const url_host* host, std::string* dest) { dest->clear(); switch (host->type) { case HOST_TYPE_DOMAIN: @@ -934,8 +934,7 @@ static url_host_type WriteHost(url_host* host, std::string* dest) { uint32_t value = host->value.ipv4; for (int n = 0; n < 4; n++) { char buf[4]; - char* buffer = buf; - snprintf(buffer, sizeof(buf), "%d", value % 256); + snprintf(buf, sizeof(buf), "%d", value % 256); dest->insert(0, buf); if (n < 3) dest->insert(0, 1, '.'); @@ -946,12 +945,12 @@ static url_host_type WriteHost(url_host* host, std::string* dest) { case HOST_TYPE_IPV6: { dest->reserve(41); *dest+= '['; - uint16_t* start = &host->value.ipv6[0]; - uint16_t* compress_pointer = + const uint16_t* start = &host->value.ipv6[0]; + const uint16_t* compress_pointer = FindLongestZeroSequence(start, 8); bool ignore0 = false; for (int n = 0; n <= 7; n++) { - uint16_t* piece = &host->value.ipv6[n]; + const uint16_t* piece = &host->value.ipv6[n]; if (ignore0 && *piece == 0) continue; else if (ignore0) @@ -962,8 +961,7 @@ static url_host_type WriteHost(url_host* host, std::string* dest) { continue; } char buf[5]; - char* buffer = buf; - snprintf(buffer, sizeof(buf), "%x", *piece); + snprintf(buf, sizeof(buf), "%x", *piece); *dest += buf; if (n < 7) *dest += ':'; @@ -980,16 +978,16 @@ static url_host_type WriteHost(url_host* host, std::string* dest) { return host->type; } -static bool ParseHost(std::string* input, +static bool ParseHost(const std::string& input, std::string* output, bool is_special, bool unicode = false) { - if (input->length() == 0) { + if (input.length() == 0) { output->clear(); return true; } url_host host{{""}, HOST_TYPE_DOMAIN}; - ParseHost(&host, input->c_str(), input->length(), is_special, unicode); + ParseHost(&host, input.c_str(), input.length(), is_special, unicode); if (host.type == HOST_TYPE_FAILED) return false; WriteHost(&host, output); @@ -1092,7 +1090,7 @@ static inline void HarvestContext(Environment* env, } // Single dot segment can be ".", "%2e", or "%2E" -static inline bool IsSingleDotSegment(std::string str) { +static inline bool IsSingleDotSegment(const std::string& str) { switch (str.size()) { case 1: return str == "."; @@ -1108,7 +1106,7 @@ static inline bool IsSingleDotSegment(std::string str) { // Double dot segment can be: // "..", ".%2e", ".%2E", "%2e.", "%2E.", // "%2e%2e", "%2E%2E", "%2e%2E", or "%2E%2e" -static inline bool IsDoubleDotSegment(std::string str) { +static inline bool IsDoubleDotSegment(const std::string& str) { switch (str.size()) { case 2: return str == ".."; @@ -1542,7 +1540,7 @@ void URL::Parse(const char* input, return; } url->flags |= URL_FLAGS_HAS_HOST; - if (!ParseHost(&buffer, &url->host, special)) { + if (!ParseHost(buffer, &url->host, special)) { url->flags |= URL_FLAGS_FAILED; return; } @@ -1569,7 +1567,7 @@ void URL::Parse(const char* input, return; } url->flags |= URL_FLAGS_HAS_HOST; - if (!ParseHost(&buffer, &url->host, special)) { + if (!ParseHost(buffer, &url->host, special)) { url->flags |= URL_FLAGS_FAILED; return; } @@ -1741,7 +1739,7 @@ void URL::Parse(const char* input, state = kPathStart; } else { std::string host; - if (!ParseHost(&buffer, &host, special)) { + if (!ParseHost(buffer, &host, special)) { url->flags |= URL_FLAGS_FAILED; return; } @@ -2104,8 +2102,7 @@ std::string URL::ToFilePath() const { #endif std::string decoded_path; for (const std::string& part : context_.path) { - std::string decoded; - PercentDecode(part.c_str(), part.length(), &decoded); + std::string decoded = PercentDecode(part.c_str(), part.length()); for (char& ch : decoded) { if (is_slash(ch)) { return "";