Skip to content

Enable shadow warnings and optional warnings-as-errors (#531) #535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bsergean merged 4 commits into from
Nov 9, 2026
+108 −116
Merged

Enable shadow warnings and optional warnings-as-errors (#531) #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56e0303
Add -Werror -Wshadow compiler flags to CMAKE_CXX_FLAGS (#531)
vaezim Dec 8, 2024
5f13fcf
Merge current master into PR 535, preserving recent socket fixes
bsergean Nov 9, 2026
b2c7e44
Scope warning flags to project targets and enable strict Linux CI
bsergean Nov 9, 2026
518b7d3
Use ostringstream oss for output-only formatting
bsergean Nov 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/unittest_linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ jobs:
- uses: actions/checkout@v1
- uses: seanmiddleditch/gha-setup-ninja@master
- name: make test
run: make -f makefile.dev test
run: make -f makefile.dev test WARNINGS_AS_ERRORS=ON
22 changes: 16 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,20 @@ if (${CMAKE_SYSTEM_NAME} MATCHES "Linux")
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
endif()

if (UNIX)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -pedantic")
endif()

if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wshorten-64-to-32")
option(IXWEBSOCKET_WARNINGS_AS_ERRORS "Treat IXWebSocket and ws warnings as errors" OFF)
set(IXWEBSOCKET_WARNING_FLAGS)
if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang" AND NOT MSVC)
if (UNIX)
list(APPEND IXWEBSOCKET_WARNING_FLAGS -Wall -Wextra -Wshadow -pedantic)
endif()
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
list(APPEND IXWEBSOCKET_WARNING_FLAGS -Wshorten-64-to-32)
endif()
if (IXWEBSOCKET_WARNINGS_AS_ERRORS)
list(APPEND IXWEBSOCKET_WARNING_FLAGS -Werror)
endif()
elseif (MSVC AND IXWEBSOCKET_WARNINGS_AS_ERRORS)
list(APPEND IXWEBSOCKET_WARNING_FLAGS /WX)
endif()

set( IXWEBSOCKET_SOURCES
Expand Down Expand Up @@ -325,6 +333,8 @@ target_include_directories(ixwebsocket PUBLIC

set_target_properties(ixwebsocket PROPERTIES PUBLIC_HEADER "${IXWEBSOCKET_HEADERS}")

target_compile_options(ixwebsocket PRIVATE ${IXWEBSOCKET_WARNING_FLAGS})

add_library(ixwebsocket::ixwebsocket ALIAS ixwebsocket)

option(IXWEBSOCKET_INSTALL "Install IXWebSocket" TRUE)
Expand Down
1 change: 1 addition & 0 deletions docs/build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Options for building:
* `-DUSE_MBED_TLS=1` will use [mbedlts](https://tls.mbed.org/) for the TLS support
* `-DUSE_WS=1` will build the ws interactive command line tool
* `-DUSE_TEST=1` will build the unittest
* `-DIXWEBSOCKET_WARNINGS_AS_ERRORS=ON` treats library and CLI compiler warnings as errors. It is off by default; `make -f makefile.dev test WARNINGS_AS_ERRORS=ON` enables it and builds both targets, as used by Linux CI. Unix GCC/Clang builds enable `-Wshadow`. Warning flags are private to these targets and do not apply to vendored dependencies or consumers.

The default CTest suite uses loopback servers and in-memory TLS sessions; it does
not require Internet access. TLS server tests generate a local CA and verify the
Expand Down
110 changes: 47 additions & 63 deletions ixwebsocket/IXHttpClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,14 +264,14 @@ namespace ix
if (!success)
{
auto errorCode = args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::CannotConnect;
std::stringstream ss;
ss << "Cannot connect to url: " << url << " / error : " << errMsg;
std::ostringstream oss;
oss << "Cannot connect to url: " << url << " / error : " << errMsg;
return std::make_shared<HttpResponse>(code,
description,
errorCode,
headers,
payload,
ss.str(),
oss.str(),
uploadSize,
downloadSize);
}
Expand All @@ -281,29 +281,23 @@ namespace ix

if (args->verbose)
{
std::stringstream ss;
ss << "Sending " << verb << " request "
<< "to " << host << ":" << port << std::endl
<< "request size: " << req.size() << " bytes" << std::endl
<< "=============" << std::endl
<< req << "=============" << std::endl
<< std::endl;
std::ostringstream oss;
oss << "Sending " << verb << " request "
<< "to " << host << ":" << port << std::endl
<< "request size: " << req.size() << " bytes" << std::endl
<< "=============" << std::endl
<< req << "=============" << std::endl
<< std::endl;

log(ss.str(), args);
log(oss.str(), args);
}

if (!_socket->writeBytes(req, isCancellationRequested))
{
auto errorCode = args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::SendError;
std::string errorMsg("Cannot send request");
return std::make_shared<HttpResponse>(code,
description,
errorCode,
headers,
payload,
errorMsg,
uploadSize,
downloadSize);
std::string msg("Cannot send request");
return std::make_shared<HttpResponse>(
code, description, errorCode, headers, payload, msg, uploadSize, downloadSize);
}

uploadSize = req.size();
Expand All @@ -315,33 +309,27 @@ namespace ix
if (!lineValid)
{
auto errorCode = args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::CannotReadStatusLine;
std::string errorMsg("Cannot retrieve status line");
return std::make_shared<HttpResponse>(code,
description,
errorCode,
headers,
payload,
errorMsg,
uploadSize,
downloadSize);
std::string msg("Cannot retrieve status line");
return std::make_shared<HttpResponse>(
code, description, errorCode, headers, payload, msg, uploadSize, downloadSize);
}

if (args->verbose)
{
std::stringstream ss;
ss << "Status line " << line;
log(ss.str(), args);
std::ostringstream oss;
oss << "Status line " << line;
log(oss.str(), args);
}

if (sscanf(line.c_str(), "HTTP/1.1 %d", &code) != 1)
{
std::string errorMsg("Cannot parse response code from status line");
std::string msg("Cannot parse response code from status line");
return std::make_shared<HttpResponse>(code,
description,
HttpErrorCode::MissingStatus,
headers,
payload,
errorMsg,
msg,
uploadSize,
downloadSize);
}
Expand All @@ -353,43 +341,37 @@ namespace ix
if (!headersValid)
{
auto errorCode = args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::HeaderParsingError;
std::string errorMsg("Cannot parse http headers");
return std::make_shared<HttpResponse>(code,
description,
errorCode,
headers,
payload,
errorMsg,
uploadSize,
downloadSize);
std::string msg("Cannot parse http headers");
return std::make_shared<HttpResponse>(
code, description, errorCode, headers, payload, msg, uploadSize, downloadSize);
}

// Redirect ?
if ((code >= 301 && code <= 308) && args->followRedirects)
{
if (headers.find("Location") == headers.end())
{
std::string errorMsg("Missing location header for redirect");
std::string msg("Missing location header for redirect");
return std::make_shared<HttpResponse>(code,
description,
HttpErrorCode::MissingLocation,
headers,
payload,
errorMsg,
msg,
uploadSize,
downloadSize);
}

if (redirects >= args->maxRedirects)
{
std::stringstream ss;
ss << "Too many redirects: " << redirects;
std::ostringstream oss;
oss << "Too many redirects: " << redirects;
return std::make_shared<HttpResponse>(code,
description,
HttpErrorCode::TooManyRedirects,
headers,
payload,
ss.str(),
oss.str(),
uploadSize,
downloadSize);
}
Expand Down Expand Up @@ -446,7 +428,7 @@ namespace ix
else if (headers.find("Transfer-Encoding") != headers.end() &&
headers["Transfer-Encoding"] == "chunked")
{
std::stringstream ss;
std::stringstream chunkSizeSs;

while (true)
{
Expand All @@ -467,13 +449,13 @@ namespace ix
}

uint64_t chunkSize;
ss.str("");
ss << std::hex << line;
ss >> chunkSize;
chunkSizeSs.str("");
chunkSizeSs << std::hex << line;
chunkSizeSs >> chunkSize;

if (args->verbose)
{
std::stringstream oss;
std::ostringstream oss;
oss << "Reading " << chunkSize << " bytes" << std::endl;
log(oss.str(), args);
}
Expand All @@ -485,11 +467,12 @@ namespace ix
isCancellationRequested);
if (!chunkResult.first)
{
auto errorCode = args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::ChunkReadError;
auto errCode =
args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::ChunkReadError;
errorMsg = "Cannot read chunk";
return std::make_shared<HttpResponse>(code,
description,
errorCode,
errCode,
headers,
payload,
errorMsg,
Expand All @@ -508,10 +491,11 @@ namespace ix

if (!lineResult.first)
{
auto errorCode = args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::ChunkReadError;
auto errCode =
args->cancel ? HttpErrorCode::Cancelled : HttpErrorCode::ChunkReadError;
return std::make_shared<HttpResponse>(code,
description,
errorCode,
errCode,
headers,
payload,
errorMsg,
Expand All @@ -528,13 +512,13 @@ namespace ix
}
else
{
std::string errorMsg("Cannot read http body");
std::string msg("Cannot read http body");
return std::make_shared<HttpResponse>(code,
description,
HttpErrorCode::CannotReadBody,
headers,
payload,
errorMsg,
msg,
uploadSize,
downloadSize);
}
Expand All @@ -548,25 +532,25 @@ namespace ix
std::string decompressedPayload;
if (!gzipDecompress(payload, decompressedPayload))
{
std::string errorMsg("Error decompressing payload");
std::string msg("Error decompressing payload");
return std::make_shared<HttpResponse>(code,
description,
HttpErrorCode::Gzip,
headers,
payload,
errorMsg,
msg,
uploadSize,
downloadSize);
}
payload = decompressedPayload;
#else
std::string errorMsg("ixwebsocket was not compiled with gzip support on");
std::string msg("ixwebsocket was not compiled with gzip support on");
return std::make_shared<HttpResponse>(code,
description,
HttpErrorCode::Gzip,
headers,
payload,
errorMsg,
msg,
uploadSize,
downloadSize);
#endif
Expand Down
1 change: 0 additions & 1 deletion ixwebsocket/IXHttpServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ namespace ix
if (std::get<0>(ret))
{
auto request = std::get<2>(ret);
std::shared_ptr<ix::HttpResponse> response;
// The Upgrade header value is case-insensitive (RFC 6455 4.2.1);
// e.g. Chrome's remote-debugging proxy sends "Upgrade: WebSocket".
const std::string& upgradeHeader = request->headers["Upgrade"];
Expand Down
4 changes: 2 additions & 2 deletions ixwebsocket/IXSocketNetwork.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ namespace ix
state->connection,
1,
65536,
^(dispatch_data_t data, nw_content_context_t, bool complete, nw_error_t error) {
^(dispatch_data_t data, nw_content_context_t, bool complete, nw_error_t readError) {
auto s = weak.lock();
if (!s) return;
std::lock_guard<std::mutex> lock(s->mutex);
Expand All @@ -349,7 +349,7 @@ namespace ix
s->error = "Cannot map received TLS data";
}
s->eof = complete;
if (error) s->error = networkError(error);
if (readError) s->error = networkError(readError);
if (!s->input.empty() || s->eof || !s->error.empty())
s->signal.notify();
else
Expand Down
24 changes: 12 additions & 12 deletions ixwebsocket/IXWebSocketHandshake.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ namespace ix
// HTTP/1.0 is too old.
if (httpVersion != "HTTP/1.1")
{
std::stringstream ss;
ss << "Expecting HTTP/1.1, got " << httpVersion << ". "
<< "Rejecting connection to " << url << ", status: " << status
<< ", HTTP Status line: " << line;
return WebSocketInitResult(false, status, ss.str());
std::ostringstream oss;
oss << "Expecting HTTP/1.1, got " << httpVersion << ". "
<< "Rejecting connection to " << url << ", status: " << status
<< ", HTTP Status line: " << line;
return WebSocketInitResult(false, status, oss.str());
}

auto result = parseHttpHeaders(_socket, isCancellationRequested);
Expand All @@ -208,11 +208,11 @@ namespace ix
// a redirection (like 301)
if (status != 101)
{
std::stringstream ss;
ss << "Expecting status 101 (Switching Protocol), got " << status
<< " status connecting to " << url << ", HTTP Status line: " << line;
std::ostringstream oss;
oss << "Expecting status 101 (Switching Protocol), got " << status
<< " status connecting to " << url << ", HTTP Status line: " << line;

return WebSocketInitResult(false, status, ss.str(), headers, path);
return WebSocketInitResult(false, status, oss.str(), headers, path);
}

// Check the presence of the connection field
Expand All @@ -230,9 +230,9 @@ namespace ix
//
if (!insensitiveStringCompare(headers["connection"], "Upgrade"))
{
std::stringstream ss;
ss << "Invalid connection value: " << headers["connection"];
return WebSocketInitResult(false, status, ss.str());
std::ostringstream oss;
oss << "Invalid connection value: " << headers["connection"];
return WebSocketInitResult(false, status, oss.str());
}

char output[29] = {};
Expand Down
2 changes: 1 addition & 1 deletion ixwebsocket/IXWebSocketTransport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ namespace ix
}
std::ptrdiff_t ret = 0;
{
std::lock_guard<std::mutex> lock(_socketMutex);
std::lock_guard<std::mutex> socketLock(_socketMutex);
ret = _socket->send((char*) _txbufPending.data() + _txbufPendingOffset,
_txbufPending.size() - _txbufPendingOffset);
}
Expand Down
Loading
Loading