Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 5, 2025

Problem

When compiling on Windows 10 using MinGW, the build fails with the following error:

error: 'close' was not declared in this scope; did you mean 'fclose'?

This occurs in src/dllama-api.cpp at line 563 and several locations in src/nn/nn-network.cpp. The issue prevents Windows users from building both dllama and dllama-api.

Root Cause

On Windows, sockets must be closed using closesocket() from the Winsock2 library, not the POSIX close() function. While the codebase already has conditional includes for Windows vs Unix socket headers, some socket close operations were not wrapped with platform-specific preprocessor directives.

Solution

This PR adds platform-specific handling for socket close operations following the same pattern already established elsewhere in the codebase:

  • Windows (#ifdef _WIN32): Uses closesocket()
  • Unix/Linux (#else): Uses close()

Changes

  • src/dllama-api.cpp: Fixed client socket close after HTTP request handling
  • src/nn/nn-network.cpp: Fixed socket close operations in:
    • closeServerSocket() function
    • NnNetwork::serve() method
    • NnNetwork::~NnNetwork() destructor

Testing

✅ Verified compilation on Linux for both dllama and dllama-api targets - all builds succeed with no errors or warnings.

Fixes #234

Related Issues

This follows the same pattern used for Windows compatibility in other parts of the codebase (e.g., lines 73, 194, 208 in nn-network.cpp).

Original prompt

please try to fix this issue: @b4rtaz/distributed-llama/issues/234


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] please try to fix this issue: @b4rtaz/distributed-llama/issues/234 Fix Windows compilation error with socket close() function Oct 5, 2025
@Copilot Copilot AI requested a review from b4rtaz October 5, 2025 17:11
Copilot finished work on behalf of b4rtaz October 5, 2025 17:11
@b4rtaz b4rtaz marked this pull request as ready for review October 7, 2025 21:02
@b4rtaz b4rtaz merged commit 21195a5 into main Oct 7, 2025
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.

The test of windows 10 failed during "make dllema"
2 participants