chore(compat): add windows portability shims and header guards by somethingwithproof · Pull Request #524 · Cacti/spine

somethingwithproof · 2026-04-08T07:33:33Z

Summary

Mechanical #ifdef _WIN32 changes preparing Spine for native Windows compilation. Zero behavioral change on POSIX. No new dependencies.

Changes

common.h -- Header guards for POSIX-only includes:

sys/wait.h, sys/socket.h, sys/select.h, netdb.h, syslog.h, arpa/inet.h, netinet/* guarded with #ifndef _WIN32
Windows alternative: winsock2.h, ws2tcpip.h, windows.h, iphlpapi.h, process.h, io.h
sys/time.h block uses plain time.h on Windows

spine.h -- Portability macros:

usleep -> Sleep, sleep -> Sleep, getpid -> _getpid, isatty -> _isatty, fileno -> _fileno
setenv -> _putenv_s, localtime_r -> localtime_s, strcasecmp -> _stricmp
CLOSE_SOCKET macro (closesocket on Windows, close on POSIX)
pid_t typedef for Windows
Windows config paths and default logfile

error.c -- Signal handling:

SIGPIPE, SIGBUS, SIGQUIT, SIGSYS guarded with #ifndef _WIN32
sigaction blocks guarded; signal() fallback works on both platforms

util.c -- Runtime:

Syslog block guarded with #ifndef _WIN32
get_time_as_double uses QueryPerformanceCounter on Windows

ping.c -- Socket I/O:

15 close() calls on sockets replaced with CLOSE_SOCKET()
read() on UDP socket uses recv() on Windows

php.c, nft_popen.c -- Build:

spawn.h include guarded with #ifndef _WIN32

Scope

This PR covers LOW and MEDIUM risk changes only. HIGH risk items (CreateProcess for process spawning, IcmpSendEcho for ICMP ping, seteuid privilege management) are deferred to a follow-up PR.

Test plan

Linux autotools build passes (no behavioral change)
Linux CMake build passes (with PR chore(build): add cmake build system and windows ci job #523)
Windows CMake build progresses further (fewer compile errors)

Mechanical #ifdef _WIN32 changes for Windows compatibility: - Guard POSIX-only headers in common.h (sys/wait, netinet/*, syslog, etc.) - Add Win32 function macros in spine.h (usleep, getpid, localtime_r, etc.) - Add CLOSE_SOCKET macro for socket/pipe distinction - Guard POSIX-only signals in error.c (SIGPIPE, SIGBUS, SIGQUIT, SIGSYS) - Guard syslog calls in util.c - Add QueryPerformanceCounter for get_time_as_double on Windows - Replace close() with CLOSE_SOCKET() for sockets in ping.c - Guard spawn.h includes in php.c and nft_popen.c Zero behavioral change on POSIX. All existing tests pass unchanged. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot

Pull request overview

This PR introduces Windows-specific header guards and portability shims to move the Spine C codebase closer to native Windows compilation, while aiming to keep POSIX behavior unchanged.

Changes:

Add _WIN32-guarded includes in common.h and POSIX-only include guards across the codebase.
Introduce Windows portability macros/types in spine.h (Sleep/usleep mappings, env/time/string shims, socket close macro).
Adjust platform-specific behavior in logging/timing (util.c), signal handling (error.c), socket I/O (ping.c), and spawn header usage (php.c, nft_popen.c).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
common.h	Adds Windows header set and guards POSIX-only headers
spine.h	Adds Windows portability shims/macros and config defaults
error.c	Guards non-Windows signals and sigaction usage
util.c	Guards syslog usage and adds Windows high-res timing path
ping.c	Replaces socket close calls with `CLOSE_SOCKET()` and uses `recv()` on Windows
php.c	Guards `<spawn.h>` include on Windows
nft_popen.c	Guards `<spawn.h>` include on Windows

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T07:42:37Z

util.c

+	QueryPerformanceFrequency(&freq);
+	QueryPerformanceCounter(&count);
+	return (double)count.QuadPart / (double)freq.QuadPart;


get_time_as_double() uses QueryPerformanceFrequency/QueryPerformanceCounter without checking for failure. If QueryPerformanceFrequency fails or returns 0, this will divide by zero. Suggest validating the return values and providing a fallback (e.g., GetTickCount64 or time-based path) when high-resolution counters are unavailable.

Suggested change

QueryPerformanceFrequency(&freq);

QueryPerformanceCounter(&count);

return (double)count.QuadPart / (double)freq.QuadPart;

if (QueryPerformanceFrequency(&freq) && freq.QuadPart != 0 &&

QueryPerformanceCounter(&count)) {

return (double)count.QuadPart / (double)freq.QuadPart;

}

return (double)GetTickCount64() / 1000.0;

Valid. Will add error check and cache the frequency in a static variable in the next iteration.

Copilot · 2026-04-08T07:42:38Z

util.c

+	LARGE_INTEGER freq, count;
+	QueryPerformanceFrequency(&freq);


QueryPerformanceFrequency is typically constant for the lifetime of the process, but this calls it on every get_time_as_double() invocation. Given how frequently get_time_as_double() is used in polling loops, consider caching the frequency (e.g., static init-on-first-use) to reduce overhead on Windows.

Suggested change

LARGE_INTEGER freq, count;

QueryPerformanceFrequency(&freq);

static LARGE_INTEGER freq;

static int freq_initialized = 0;

LARGE_INTEGER count;

if (!freq_initialized) {

QueryPerformanceFrequency(&freq);

freq_initialized = 1;

}

Agreed. Will cache in a static local.

Copilot · 2026-04-08T07:42:38Z

common.h

+#include <windows.h>
+#include <iphlpapi.h>
+#include <process.h>
+#include <io.h>


Winsock requires calling WSAStartup() (and WSACleanup() on shutdown) before using socket()/select()/closesocket(). I don’t see any WSAStartup call in the codebase, so even if this compiles the network paths will fail at runtime on native Windows. Consider adding a small platform init/teardown routine (e.g., in spine.c startup) that runs when _WIN32 is set.

Suggested change

#include <io.h>

#include <io.h>

static void spine_winsock_cleanup(void) {

WSACleanup();

}

static BOOL CALLBACK spine_winsock_init_once(PINIT_ONCE init_once, PVOID parameter, PVOID *context) {

WSADATA wsa_data;

int result;

(void)init_once;

(void)parameter;

(void)context;

result = WSAStartup(MAKEWORD(2, 2), &wsa_data);

if (result != 0) {

WSASetLastError(result);

return FALSE;

}

atexit(spine_winsock_cleanup);

return TRUE;

}

static int spine_winsock_init(void) {

static INIT_ONCE spine_winsock_init_once_control = INIT_ONCE_STATIC_INIT;

return InitOnceExecuteOnce(&spine_winsock_init_once_control, spine_winsock_init_once, NULL, NULL) ? 0 : SOCKET_ERROR;

}

static SOCKET spine_socket(int af, int type, int protocol) {

if (spine_winsock_init() != 0) {

return INVALID_SOCKET;

}

return socket(af, type, protocol);

}

static int spine_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, const struct timeval *timeout) {

if (spine_winsock_init() != 0) {

return SOCKET_ERROR;

}

return select(nfds, readfds, writefds, exceptfds, timeout);

}

static int spine_closesocket(SOCKET s) {

if (spine_winsock_init() != 0) {

return SOCKET_ERROR;

}

return closesocket(s);

}

#define socket(af, type, protocol) spine_socket((af), (type), (protocol))

#define select(nfds, readfds, writefds, exceptfds, timeout) spine_select((nfds), (readfds), (writefds), (exceptfds), (timeout))

#define closesocket(s) spine_closesocket((s))

WSAStartup is handled by Net-SNMP SOCK_STARTUP macro already called in spine.c. Documenting this dependency.

Copilot · 2026-04-08T07:42:38Z

php.c


 #include "common.h"
 #include "spine.h"
+#ifndef _WIN32


This guards out <spawn.h> on _WIN32, but php.c still uses posix_spawn()/posix_spawn_file_actions_t later. With _WIN32 defined, that will typically fail to compile due to missing type/function declarations. Either include the appropriate header based on a feature test (e.g., HAVE_SPAWN_H) or #ifdef out the posix_spawn-based implementation on Windows until a replacement is added.

Suggested change

#ifndef _WIN32

#ifdef HAVE_SPAWN_H

Correct. The spawn.h guard is a prerequisite. The CreateProcess replacement is tracked separately in the libuv migration work.

Copilot · 2026-04-08T07:42:38Z

nft_popen.c


 #include "common.h"
 #include "spine.h"
+#ifndef _WIN32


This guards out <spawn.h> on _WIN32, but nft_popen() uses posix_spawn()/posix_spawn_file_actions_t later. That will generally not compile on Windows because the types/prototypes won’t be declared. Suggest using a feature-test macro (HAVE_SPAWN_H) rather than _WIN32 for the include, or wrapping the posix_spawn implementation in #ifndef _WIN32 until a Windows path is added.

Suggested change

#ifndef _WIN32

#ifdef HAVE_SPAWN_H

Same as above. Guard-only for now, full replacement deferred.

Copilot · 2026-04-08T07:42:39Z

spine.h

+/* Type portability */
+typedef int pid_t;


The Windows shim unconditionally typedefs pid_t as int. However, common.h may already include <sys/types.h> (when HAVE_UNISTD_H is true), which typically defines pid_t; this can cause a redefinition compile error on some Windows toolchains (e.g., MinGW/MSYS2). Prefer only defining pid_t when it’s not already provided by the environment (feature-test macro / guard) or relying on the platform headers instead.

Suggested change

/* Type portability */

typedef int pid_t;

/* Type portability */

#if !defined(_PID_T_) && !defined(_PID_T_DEFINED) && !defined(__pid_t_defined)

typedef int pid_t;

#endif

Will add a guard: typedef only when pid_t is not already defined.

Copilot · 2026-04-08T07:42:39Z

ping.c


 						if (return_code == -1 && (errno == EHOSTUNREACH || errno == ECONNRESET || errno == ECONNREFUSED)) {
 							if (is_debug_device(host->id)) {


Under _WIN32 you call recv(), but this error check still relies on errno and POSIX error codes. Winsock reports socket errors via WSAGetLastError() and WSA* constants (and recv returns SOCKET_ERROR), so this condition won’t behave correctly on Windows. Consider using WSAGetLastError()/WSAECONNRESET/WSAECONNREFUSED/WSAEHOSTUNREACH in the _WIN32 path.

Valid. Will use WSAGetLastError() and Winsock error codes in the _WIN32 path.

Copilot · 2026-04-08T07:42:39Z

ping.c

+		if (udp_socket != -1) CLOSE_SOCKET(udp_socket);
 		return HOST_DOWN;


On Windows, socket handles are SOCKET and the invalid sentinel is INVALID_SOCKET (and errors are SOCKET_ERROR), not -1. This branch still uses udp_socket != -1, which is not type-correct for Winsock and can break on 64-bit. Consider using INVALID_SOCKET/SOCKET_ERROR (and a SOCKET-typed variable) under _WIN32.

Valid. CLOSE_SOCKET handles closesocket vs close, but SOCKET type and INVALID_SOCKET sentinel need addressing. Will fix in follow-up.

spine.h depends on types from common.h (MYSQL, pid_t, size_t, pthread types, RESULTS_BUFFER from config.h). Add a compile-time guard that produces a clear error if spine.h is included without common.h, rather than cascading type errors. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof · 2026-04-11T11:02:27Z

Review note: this PR should merge before #523 (CMake build). #523's platform_*.c files overlap with the shims added here; rebase #523 on this after merge.

Copilot AI review requested due to automatic review settings April 8, 2026 07:33

Copilot started reviewing on behalf of somethingwithproof April 8, 2026 07:34 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

somethingwithproof added 2 commits April 8, 2026 00:45

fix(ping): use portable socket handle type

8d42913

somethingwithproof mentioned this pull request Apr 11, 2026

chore(build): add cmake build system and windows ci job #523

Open

4 tasks

-	LARGE_INTEGER freq, count;
-	QueryPerformanceFrequency(&freq);
+	static LARGE_INTEGER freq;
+	static int freq_initialized = 0;
+	LARGE_INTEGER count;
+	if (!freq_initialized) {
+		QueryPerformanceFrequency(&freq);
+		freq_initialized = 1;
+	}

-#include <io.h>
+#include <io.h>
+static void spine_winsock_cleanup(void) {
+	WSACleanup();
+}
+static BOOL CALLBACK spine_winsock_init_once(PINIT_ONCE init_once, PVOID parameter, PVOID *context) {
+	WSADATA wsa_data;
+	int     result;
+	(void)init_once;
+	(void)parameter;
+	(void)context;
+	result = WSAStartup(MAKEWORD(2, 2), &wsa_data);
+	if (result != 0) {
+		WSASetLastError(result);
+		return FALSE;
+	}
+	atexit(spine_winsock_cleanup);
+	return TRUE;
+}
+static int spine_winsock_init(void) {
+	static INIT_ONCE spine_winsock_init_once_control = INIT_ONCE_STATIC_INIT;
+	return InitOnceExecuteOnce(&spine_winsock_init_once_control, spine_winsock_init_once, NULL, NULL) ? 0 : SOCKET_ERROR;
+}
+static SOCKET spine_socket(int af, int type, int protocol) {
+	if (spine_winsock_init() != 0) {
+		return INVALID_SOCKET;
+	}
+	return socket(af, type, protocol);
+}
+static int spine_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, const struct timeval *timeout) {
+	if (spine_winsock_init() != 0) {
+		return SOCKET_ERROR;
+	}
+	return select(nfds, readfds, writefds, exceptfds, timeout);
+}
+static int spine_closesocket(SOCKET s) {
+	if (spine_winsock_init() != 0) {
+		return SOCKET_ERROR;
+	}
+	return closesocket(s);
+}
+#define socket(af, type, protocol) spine_socket((af), (type), (protocol))
+#define select(nfds, readfds, writefds, exceptfds, timeout) spine_select((nfds), (readfds), (writefds), (exceptfds), (timeout))
+#define closesocket(s) spine_closesocket((s))


		if (return_code == -1 && (errno == EHOSTUNREACH \|\| errno == ECONNRESET \|\| errno == ECONNREFUSED)) {
		if (is_debug_device(host->id)) {

		if (udp_socket != -1) CLOSE_SOCKET(udp_socket);
		return HOST_DOWN;

Conversation

somethingwithproof commented Apr 8, 2026

Summary

Changes

Scope

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

somethingwithproof commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants