Skip to content

Fix armv7a and windows build - #17

Open
Keeqler wants to merge 1 commit into
vtnerd:mainfrom
MAGICGrants:build-fix
Open

Keeqler wants to merge 1 commit into
vtnerd:mainfrom
MAGICGrants:build-fix

Conversation

@Keeqler

@Keeqler Keeqler commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Comment thread src/net/http.cpp
// size_t is 32-bit on some targets (armv7): a Content-Length that
// can't fit is not a body we could hold, so reject it rather than
// truncate the reserve.
if constexpr (std::numeric_limits<std::uint64_t>::max() > std::numeric_limits<std::size_t>::max())

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check shouldn't be needed, as the compiler should be able to optimize away the check below at compile-time.

@@ -28,10 +28,11 @@

#include "subaddress_account.h"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this fixing? Some WIN32 issue?

Comment thread CMakeLists.txt
# Keep windows.h from pulling in WinSock1: boost::asio needs WinSock2, and
# headers that include windows.h first would otherwise make its
# socket_types.hpp fail with "WinSock.h has already been included".
add_compile_definitions(WIN32_LEAN_AND_MEAN)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this add to all projects/executables declared after this? I think we want target_add_compile_definitions(lwsf-api WIN32_LEAN_AND_MEAN) in the src folder.

@vtnerd

vtnerd commented Sep 13, 2026

Copy link
Copy Markdown
Owner

The windows test also failed, and it's likely due to the changes introduced here.

I'm not sure why that WIN32_LEAN_AND_MEAN guard is needed, as we have a working windows build ... ? Or are you using MSVC .. ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants