uNetworking / uWebSockets

Simple, secure & standards compliant web server for the most demanding of applications
Apache License 2.0
17.24k stars 1.75k forks source link

Fix pedantic warnings #1080

Closed major-mayer closed 3 years ago

major-mayer commented 4 years ago

Hi, first of all, thanks for this great library. When i compile it, i noticed that it produces one warning, which spams the logs a bit, because there are many references after it.

lib\uWebSockets\src\WebSocketContext.h(326,47): warning C4018: '<': signed/unsigned mismatch

It's no big deal an not crucial, but i just wanted to inform you that the warning is produced in case you didn't notice.

I am using MSVC 19 (C++20 standard). Greetings

yesudeep commented 4 years ago

Thank you for the awesome library. Adding another mismatched sign comparison reported in a different file by GCC 10.1.1 on Linux:

INFO: From Compiling third_party/cc/test/uwebsockets_example.cc:
In file included from bazel-out/k8-fastbuild/bin/external/uwebsockets/_virtual_includes/uwebsockets/uwebsockets/HttpParser.h:31,
                 from bazel-out/k8-fastbuild/bin/external/uwebsockets/_virtual_includes/uwebsockets/uwebsockets/HttpResponseData.h:23,
                 from bazel-out/k8-fastbuild/bin/external/uwebsockets/_virtual_includes/uwebsockets/uwebsockets/HttpContext.h:25,
                 from bazel-out/k8-fastbuild/bin/external/uwebsockets/_virtual_includes/uwebsockets/uwebsockets/App.h:24,
                 from third_party/cc/test/uwebsockets_example.cc:7:
bazel-out/k8-fastbuild/bin/external/uwebsockets/_virtual_includes/uwebsockets/uwebsockets/ProxyParser.h: In member function 'std::pair<bool, unsigned int> uWS::ProxyParser::parse(std::string_view)':
bazel-out/k8-fastbuild/bin/external/uwebsockets/_virtual_includes/uwebsockets/uwebsockets/ProxyParser.h:136:27: warning: comparison of integer expressions of different signedness: 'std::basic_string_view<char>::size_type' {aka 'long unsigned int'} and 'int' [-Wsign-compare]
  136 |         if (data.length() < 16 + hostLength) {
      |             ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
ghost commented 4 years ago

If you enable -Wsign-conversion you'll see about 60-90 of those. The thing is, these warnings are not default in GCC or Clang, so I haven't put any time into fixing them.

Then we have the time when someone with very strong opinions about warnings made a PR to "fix" them, only to mess up logic and break the whole project. So I take caution rushing to "fix" non-default warnings just for the sake of it. I would like to spend some time and go over them, but that takes time.

ghost commented 3 years ago

Alright - we now compile with no warnings for:

The recently fixed signed/unsigned warnings are actually part of -Wextra which is even more pedantic than -Wall so by fixing the remaining issues in -Wextra we could enable that setting by default. There are only like 20 warnings to fix in this category.

ghost commented 3 years ago

-Wextra now compile without any warnings