Integer literals are signed, so anything + 1 is signed: [205/6600] Building CXX object Source/WTF/wtf/CMakeFiles/WTF.dir/URL.cpp.o ../../Source/WTF/wtf/URL.cpp: In member function ‘unsigned int WTF::URL::pathStart() const’: ../../Source/WTF/wtf/URL.cpp:110:15: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare] 110 | if (start == m_schemeEnd + 1 | ~~~~~~^~~~~~~~~~~~~~~~~~ [206/6600] Building CXX object Source/WTF/wtf/CMakeFiles/WTF.dir/URLParser.cpp.o ../../Source/WTF/wtf/URLParser.cpp: In member function ‘bool WTF::URLParser::needsNonSpecialDotSlash() const’: ../../Source/WTF/wtf/URLParser.cpp:2501:22: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare] 2501 | && pathStart == m_url.m_schemeEnd + 1 | ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ We could use unsigned literals: diff --git a/Source/WTF/wtf/URL.cpp b/Source/WTF/wtf/URL.cpp index ae0324da9fc3..cf2d6bf3538c 100644 --- a/Source/WTF/wtf/URL.cpp +++ b/Source/WTF/wtf/URL.cpp @@ -107,7 +107,7 @@ bool URL::hasSpecialScheme() const unsigned URL::pathStart() const { unsigned start = m_hostEnd + m_portLength; - if (start == m_schemeEnd + 1 + if (start == m_schemeEnd + 1u && start + 1 < m_string.length() && m_string[start] == '/' && m_string[start + 1] == '.') start += 2; diff --git a/Source/WTF/wtf/URLParser.cpp b/Source/WTF/wtf/URLParser.cpp index 8bbb2c0149ca..22db53f93761 100644 --- a/Source/WTF/wtf/URLParser.cpp +++ b/Source/WTF/wtf/URLParser.cpp @@ -2498,7 +2498,7 @@ bool URLParser::needsNonSpecialDotSlash() const { auto pathStart = m_url.m_hostEnd + m_url.m_portLength; return !m_urlIsSpecial - && pathStart == m_url.m_schemeEnd + 1 + && pathStart == m_url.m_schemeEnd + 1u && pathStart + 1 < m_url.m_string.length() && m_url.m_string[pathStart] == '/' && m_url.m_string[pathStart + 1] == '/'; Or we could use casts: diff --git a/Source/WTF/wtf/URL.cpp b/Source/WTF/wtf/URL.cpp index ae0324da9fc3..b94e99759fb2 100644 --- a/Source/WTF/wtf/URL.cpp +++ b/Source/WTF/wtf/URL.cpp @@ -107,7 +107,7 @@ bool URL::hasSpecialScheme() const unsigned URL::pathStart() const { unsigned start = m_hostEnd + m_portLength; - if (start == m_schemeEnd + 1 + if (start == static_cast<unsigned>(m_schemeEnd + 1) && start + 1 < m_string.length() && m_string[start] == '/' && m_string[start + 1] == '.') start += 2; diff --git a/Source/WTF/wtf/URLParser.cpp b/Source/WTF/wtf/URLParser.cpp index 8bbb2c0149ca..be7ab18c4ca3 100644 --- a/Source/WTF/wtf/URLParser.cpp +++ b/Source/WTF/wtf/URLParser.cpp @@ -2498,7 +2498,7 @@ bool URLParser::needsNonSpecialDotSlash() const { auto pathStart = m_url.m_hostEnd + m_url.m_portLength; return !m_urlIsSpecial - && pathStart == m_url.m_schemeEnd + 1 + && pathStart == static_cast<unsigned>(m_url.m_schemeEnd + 1) && pathStart + 1 < m_url.m_string.length() && m_url.m_string[pathStart] == '/' && m_url.m_string[pathStart + 1] == '/'; No strong preference from me, but I'll attach a patch that uses casts since I think it's slightly more clear.
Created attachment 411060 [details] Patch
Comment on attachment 411060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411060&action=review > Source/WTF/wtf/URL.cpp:110 > + if (start == static_cast<unsigned>(m_schemeEnd + 1) Doesn’t make sense to me. Isn’t m_schemeEnd an unsigned already? Is there something about being a bitfield that makes things become signed?
Comment on attachment 411060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411060&action=review >> Source/WTF/wtf/URL.cpp:110 >> + if (start == static_cast<unsigned>(m_schemeEnd + 1) > > Doesn’t make sense to me. Isn’t m_schemeEnd an unsigned already? Is there something about being a bitfield that makes things become signed? Oh, you said that using "1" rather than "1U" makes it signed. I prefer 1U over 1u and certainly over static_cast<unsigned>.
1U it is, then
Created attachment 411112 [details] Patch
Comment on attachment 411112 [details] Patch Thanks.
Committed r268362: <https://trac.webkit.org/changeset/268362> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411112 [details].
<rdar://problem/70213551>