From 09c9e82a5ca2c8d83b36b36aacb7aa389403fad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:28:44 +0100 Subject: [PATCH 01/25] ocamlmerlin.c: consistent definition of PATHSZ --- src/frontend/ocamlmerlin/ocamlmerlin.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 6aafd6483..d4f00e20b 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -21,7 +21,6 @@ #endif #ifdef _MSC_VER typedef SSIZE_T ssize_t; -#define PATH_MAX MAX_PATH #ifndef _UCRT #define snprintf _snprintf #endif @@ -92,7 +91,11 @@ static void failwith(const char *msg) exit(EXIT_FAILURE); } +#ifdef _WIN32 +#define PATHSZ (MAX_PATH+1) +#else #define PATHSZ (PATH_MAX+1) +#endif /* On Linux, sun_path size is 108 bytes. On macOS it's 104. From d3bcd918f3308bccca178371470a0c308c1289c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:17 +0100 Subject: [PATCH 02/25] ocamlmerlin.c: improve location of socket path On Windows, use [GetTempPath2][] if available or [GetTempPath][] to retrieve the temporary directory path. On other systems, follow the [XDG Base Directory Specification][XDG] and put the socket file in the XDG runtime directory. It is indicated by the `XDG_RUNTIME_DIR` environment variable, that is always set on platforms implementing the spec. Otherwise, fallback to `TMPDIR` or `/tmp`. [GetTempPath2]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppath2a [GetTempPath2]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppatha [XDG]: https://specifications.freedesktop.org/basedir/latest/ --- src/frontend/ocamlmerlin/ocamlmerlin.c | 29 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index d4f00e20b..d439e87d0 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -113,12 +113,29 @@ static void failwith(const char *msg) static const char *path_socketdir(void) { - static const char *tmpdir = NULL; - if (tmpdir == NULL) - tmpdir = getenv("TMPDIR"); - if (tmpdir == NULL) - tmpdir = "/tmp"; - return tmpdir; +#ifdef _WIN32 + static char dir[MAX_PATH+1] = { 0 }; + if (dir[0] == 0) { + DWORD (WINAPI *pGetTempPath2A)(DWORD, LPSTR) = + (DWORD (WINAPI *)(DWORD, LPSTR)) + (void (WINAPI *)(void)) + GetProcAddress(GetModuleHandle("KERNEL32.DLL"), "GetTempPath2A"); + DWORD rc; + if (pGetTempPath2A != NULL) + rc = pGetTempPath2A(_countof(dir), dir); + else + rc = GetTempPathA(_countof(dir), dir); + if (rc == 0) + failwith("could not get temporary path"); + } +#else + static const char *dir = NULL; + if (dir == NULL && + (dir = getenv("XDG_RUNTIME_DIR")) == NULL && + (dir = getenv("TMPDIR")) == NULL) + dir = "/tmp"; +#endif + return dir; } #ifdef _WIN32 From 61549c19af22d1991f8bcc9713b3219dc29f5dcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:17 +0100 Subject: [PATCH 03/25] Improve passing arrays as parameters In C, arrays passed as arguments to functions decay into pointers, and the size information is lost. This means all of these are equivalent: void f(char *s); void f(char s[]); void f(char s[3615]); One way to partially retain this information is to use `static n` with `n` being the array size: the compiler now knows that the first `n` elements after the pointed address can be accessed. This C99 feature is unfortunately not supported by MSVC (but supported by clang-cl). The compiler uses this info to check that the caller supplies a long enough buffer to the callee. The size information is not used inside the callee. --- src/frontend/ocamlmerlin/ocamlmerlin.c | 16 +++++++++++----- src/platform/os_ipc_stub.c | 8 +++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index d439e87d0..16519daa4 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -47,6 +47,12 @@ typedef SSIZE_T ssize_t; #include #endif +#if !(defined(_MSC_VER) && !defined(__clang__)) +#define ATLEAST static +#else +#define ATLEAST +#endif + /** Portability information **/ /* Determine OS, http://stackoverflow.com/questions/6649936 @@ -141,7 +147,7 @@ static const char *path_socketdir(void) #ifdef _WIN32 /** Deal with Windows IPC **/ -static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, HANDLE fds[3]) +static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, HANDLE fds[ATLEAST 3]) { DWORD dwNumberOfBytesWritten; if (!WriteFile(hPipe, fds, 3 * sizeof(HANDLE), &dwNumberOfBytesWritten, NULL) || dwNumberOfBytesWritten != 3 * sizeof(HANDLE)) @@ -153,7 +159,7 @@ static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, HANDLE fds #else /** Deal with UNIX IPC **/ -static void ipc_send(int fd, unsigned char *buffer, size_t len, int fds[3]) +static void ipc_send(int fd, unsigned char *buffer, size_t len, int fds[ATLEAST 3]) { char msg_control[CMSG_SPACE(3 * sizeof(int))]; struct iovec iov = { .iov_base = buffer, .iov_len = len }; @@ -506,7 +512,7 @@ static char ocamlmerlin_server[] = "ocamlmerlin-server.exe"; static char ocamlmerlin_server[] = "ocamlmerlin-server"; #endif -static void compute_merlinpath(char merlin_path[PATHSZ], const char *argv0, struct stat *st) +static void compute_merlinpath(char merlin_path[ATLEAST PATHSZ], const char *argv0, struct stat *st) { char argv0_dirname[PATHSZ]; size_t strsz; @@ -593,9 +599,9 @@ LPSTR retrieve_user_sid_string() return usidstr; } -static void compute_socketname(char socketname[PATHSZ], char eventname[PATHSZ], const char merlin_path[PATHSZ]) +static void compute_socketname(char socketname[ATLEAST PATHSZ], char eventname[ATLEAST PATHSZ], const char merlin_path[ATLEAST PATHSZ]) #else -static void compute_socketname(char socketname[SOCKSZ], struct stat *st) +static void compute_socketname(char socketname[ATLEAST SOCKSZ], struct stat *st) #endif { #ifdef _WIN32 diff --git a/src/platform/os_ipc_stub.c b/src/platform/os_ipc_stub.c index a95fe4754..465cdb7ec 100644 --- a/src/platform/os_ipc_stub.c +++ b/src/platform/os_ipc_stub.c @@ -30,6 +30,12 @@ typedef SSIZE_T ssize_t; #include #include +#if !(defined(_MSC_VER) && !defined(__clang__)) +#define ATLEAST static +#else +#define ATLEAST +#endif + #ifdef _MSC_VER extern __declspec(dllimport) char **environ; #else @@ -75,7 +81,7 @@ static unsigned char buffer[BUFFER_SIZE]; #define unbyte(x,n) (((unsigned char)x) << (n * 8)) -static ssize_t recv_buffer(int fd, int fds[3]) +static ssize_t recv_buffer(int fd, int fds[ATLEAST 3]) { char msg_control[CMSG_SPACE(3 * sizeof(int))]; struct iovec iov = { .iov_base = buffer, .iov_len = sizeof(buffer) }; From f8debc1d7f3b06aa1624031174cae36b190394dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:17 +0100 Subject: [PATCH 04/25] ocamlmerlin.c: improve declaration of retrieve_user_sid_string - make the function static; - don't use K&R-style declaration, compilers now reject it before C23. After C23 `f(void)` is similar to `f()`. --- src/frontend/ocamlmerlin/ocamlmerlin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 16519daa4..aa96ef5c4 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -560,7 +560,7 @@ static void compute_merlinpath(char merlin_path[ATLEAST PATHSZ], const char *arg #ifdef _WIN32 /* May return NULL */ -LPSTR retrieve_user_sid_string() +static LPSTR retrieve_user_sid_string(void) { LPSTR usidstr; HANDLE process_token; From 2c8acdfdaba630be124e5e5c6e9ee2d2fc71e491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:17 +0100 Subject: [PATCH 05/25] ocamlmerlin.c: use centralized exit of function with gotos pattern --- src/frontend/ocamlmerlin/ocamlmerlin.c | 31 +++++++++----------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index aa96ef5c4..ffa4d4d7b 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -562,7 +562,7 @@ static void compute_merlinpath(char merlin_path[ATLEAST PATHSZ], const char *arg /* May return NULL */ static LPSTR retrieve_user_sid_string(void) { - LPSTR usidstr; + LPSTR usidstr = NULL; HANDLE process_token; if ( ! OpenProcessToken( GetCurrentProcess(), TOKEN_QUERY, &process_token ) ) return NULL; @@ -570,32 +570,23 @@ static LPSTR retrieve_user_sid_string(void) DWORD sid_buffer_size; if ( ! GetTokenInformation(process_token, TokenUser, NULL, 0, &sid_buffer_size ) && ( GetLastError() != ERROR_INSUFFICIENT_BUFFER ) ) - { - CloseHandle(process_token); - return NULL; - } + goto close_process_token; - TOKEN_USER * token_user_ptr = (PTOKEN_USER) malloc(sid_buffer_size); - if ( ! token_user_ptr ) - { - CloseHandle( process_token); - return NULL; - } + TOKEN_USER * token_user = (PTOKEN_USER) malloc(sid_buffer_size); + if ( ! token_user ) + goto close_process_token; - if ( ! GetTokenInformation(process_token, TokenUser, token_user_ptr, + if ( ! GetTokenInformation(process_token, TokenUser, token_user, sid_buffer_size, &sid_buffer_size)) - { - free(token_user_ptr); - CloseHandle(process_token); - return NULL; - } + goto free_token_user; - if (! ConvertSidToStringSid(token_user_ptr->User.Sid, &usidstr)) + if (! ConvertSidToStringSid(token_user->User.Sid, &usidstr)) usidstr = NULL; - free(token_user_ptr); + free_token_user: + free(token_user); + close_process_token: CloseHandle(process_token); - return usidstr; } From 99883e8be76d1cb50efaf5c48dfee7920dcbf9e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:17 +0100 Subject: [PATCH 06/25] ocamlmerlin.c: don't hard-code size of unix domain socket path Quoting POSIX 2024: > The size of sun_path is required to be constant, but intentionally > does not have a specified name for that constant. Historically, > different implementations used different sizes. For example, 4.3 BSD > used a size of 108, and 4.4 BSD used a size of 104. Since most > implementations originate from BSD versions, the size is typically > in the range 92 to 108. An application can deduce the size by using > `sizeof(((struct sockaddr_un *)0)->sun_path)`. --- src/frontend/ocamlmerlin/ocamlmerlin.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index ffa4d4d7b..5b0e15891 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -99,16 +99,12 @@ static void failwith(const char *msg) #ifdef _WIN32 #define PATHSZ (MAX_PATH+1) +#define SOCKSZ (MAX_PATH+1) #else #define PATHSZ (PATH_MAX+1) +#define SOCKSZ (sizeof(((struct sockaddr_un *)0)->sun_path) - sizeof("./")) #endif -/* On Linux, sun_path size is 108 bytes. - On macOS it's 104. - We use 102 buffer, as we later append './' -*/ -#define SOCKSZ (102) - #define BEGIN_PROTECTCWD \ { char previous_cwd[PATHSZ]; \ if (!getcwd(previous_cwd, PATHSZ)) previous_cwd[0] = '\0'; @@ -291,7 +287,7 @@ static int connect_socket(const char *socketname, int fail) /* Return from chdir is ignored */ err = chdir(path_socketdir()); address.sun_family = AF_UNIX; - snprintf(address.sun_path, 104, "./%s", socketname); + snprintf(address.sun_path, sizeof(address.sun_path), "./%s", socketname); address_len = strlen(address.sun_path) + sizeof(address.sun_family) + 1; NO_EINTR(err, connect(sock, (struct sockaddr*)&address, address_len)); @@ -385,7 +381,7 @@ static void start_server(const char *socketname, const char* ignored, const char /* Return from chdir is ignored */ err = chdir(path_socketdir()); address.sun_family = AF_UNIX; - snprintf(address.sun_path, 104, "./%s", socketname); + snprintf(address.sun_path, sizeof(address.sun_path), "./%s", socketname); address_len = strlen(address.sun_path) + sizeof(address.sun_family) + 1; unlink(address.sun_path); From 860a67c1c8d6e142ce1ddc75c8c17e70fca60919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 07/25] ocamlmerlin.c: cleanly separate Windows and non-Windows code --- src/frontend/ocamlmerlin/ocamlmerlin.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 5b0e15891..bd64d3d5e 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -587,11 +587,7 @@ static LPSTR retrieve_user_sid_string(void) } static void compute_socketname(char socketname[ATLEAST PATHSZ], char eventname[ATLEAST PATHSZ], const char merlin_path[ATLEAST PATHSZ]) -#else -static void compute_socketname(char socketname[ATLEAST SOCKSZ], struct stat *st) -#endif { -#ifdef _WIN32 BY_HANDLE_FILE_INFORMATION info; LPSTR user_sid_string; HANDLE hFile = CreateFile(merlin_path, FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); @@ -613,14 +609,18 @@ static void compute_socketname(char socketname[ATLEAST SOCKSZ], struct stat *st) "\\\\.\\pipe\\%s", eventname); LocalFree(user_sid_string); +} + #else +static void compute_socketname(char socketname[ATLEAST SOCKSZ], struct stat *st) +{ snprintf(socketname, SOCKSZ, "ocamlmerlin_%llu_%llu_%llu.socket", (unsigned long long)getuid(), (unsigned long long)st->st_dev, (unsigned long long)st->st_ino); -#endif } +#endif /* Main */ From 31b3426a09e544172771fe3fb881295e5c7d6e74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 08/25] ocamlmerlin.c: improve initialization of struct sockaddr_un See also [`sys/un.h`][1] and [unix(7)][2]. [1]: https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/sys_un.h.html [2]: https://man7.org/linux/man-pages/man7/unix.7.html --- src/frontend/ocamlmerlin/ocamlmerlin.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index bd64d3d5e..7c14a34eb 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -282,13 +283,14 @@ static int connect_socket(const char *socketname, int fail) BEGIN_PROTECTCWD struct sockaddr_un address; - int address_len; + socklen_t address_len; + memset(&address, 0, sizeof(address)); /* Return from chdir is ignored */ err = chdir(path_socketdir()); address.sun_family = AF_UNIX; snprintf(address.sun_path, sizeof(address.sun_path), "./%s", socketname); - address_len = strlen(address.sun_path) + sizeof(address.sun_family) + 1; + address_len = offsetof(struct sockaddr_un, sun_path) + strlen(address.sun_path) + 1; NO_EINTR(err, connect(sock, (struct sockaddr*)&address, address_len)); END_PROTECTCWD @@ -376,13 +378,14 @@ static void start_server(const char *socketname, const char* ignored, const char BEGIN_PROTECTCWD struct sockaddr_un address; - int address_len; + socklen_t address_len; + memset(&address, 0, sizeof(address)); /* Return from chdir is ignored */ err = chdir(path_socketdir()); address.sun_family = AF_UNIX; snprintf(address.sun_path, sizeof(address.sun_path), "./%s", socketname); - address_len = strlen(address.sun_path) + sizeof(address.sun_family) + 1; + address_len = offsetof(struct sockaddr_un, sun_path) + strlen(address.sun_path) + 1; unlink(address.sun_path); NO_EINTR(err, bind(sock, (struct sockaddr*)&address, address_len)); From c5829724838acf235f359250d5b05743a7aacbbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 09/25] Always use __declspec(import) on environ on Windows It works with mingw-w64 too. warning: '__p__environ' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes] https://gcc.gnu.org/onlinedocs/gcc/Microsoft-Windows-Variable-Attributes.html#index-dllimport-variable-attribute --- src/frontend/ocamlmerlin/ocamlmerlin.c | 2 +- src/platform/os_ipc_stub.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 7c14a34eb..bc702b900 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -218,7 +218,7 @@ static void append_argument(unsigned char *buffer, size_t len, ssize_t *pos, con *pos = j; } -#ifdef _MSC_VER +#ifdef _WIN32 extern __declspec(dllimport) char **environ; #else extern char **environ; diff --git a/src/platform/os_ipc_stub.c b/src/platform/os_ipc_stub.c index 465cdb7ec..6f7384129 100644 --- a/src/platform/os_ipc_stub.c +++ b/src/platform/os_ipc_stub.c @@ -36,7 +36,7 @@ typedef SSIZE_T ssize_t; #define ATLEAST #endif -#ifdef _MSC_VER +#ifdef _WIN32 extern __declspec(dllimport) char **environ; #else extern char **environ; From cac70c8b3f75c38bdbed00ff25712653d834b545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 10/25] ocamlmerlin.c: fix unused variable warnings --- src/frontend/ocamlmerlin/ocamlmerlin.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index bc702b900..5d8b3ab60 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -313,7 +313,7 @@ static void start_server(const char *socketname, const char* eventname, const ch PROCESS_INFORMATION pi; STARTUPINFO si; HANDLE hEvent = CreateEvent(NULL, FALSE, FALSE, eventname); - DWORD dwResult; + sprintf(buf, "%s server %s %s", exec_path, socketname, eventname); ZeroMemory(&si, sizeof(si)); si.cb = sizeof(si); @@ -331,7 +331,7 @@ static void start_server(const char *socketname, const char* eventname, const ch failwith_perror("execlp"); } #else -static void make_daemon(int sock) +static void make_daemon(void) { /* On success: The child process becomes session leader */ if (setsid() < 0) @@ -349,13 +349,6 @@ static void make_daemon(int sock) if (chdir("/") != 0) failwith_perror("chdir"); - //int x; - //for (x = sysconf(_SC_OPEN_MAX); x>2; x--) - //{ - // if (x != sock) - // close(x); - //} - pid_t child = fork(); signal(SIGHUP, SIG_IGN); @@ -370,6 +363,7 @@ static void make_daemon(int sock) static void start_server(const char *socketname, const char* ignored, const char *exec_path) { + (void) ignored; int sock = socket(PF_UNIX, SOCK_STREAM, 0); if (sock == -1) failwith_perror("socket"); @@ -405,7 +399,7 @@ static void start_server(const char *socketname, const char* ignored, const char if (child == 0) { - make_daemon(sock); + make_daemon(); char socket_fd[50], socket_path[PATHSZ]; sprintf(socket_fd, "%d", sock); From ac648d9a71dee569b0ebb4618891ae6f424c0f27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 11/25] ocamlmerlin.c: use the cool _Countof op from C2y to count the number of elements in an array. --- src/frontend/ocamlmerlin/ocamlmerlin.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 5d8b3ab60..278d0598b 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -54,6 +54,19 @@ typedef SSIZE_T ssize_t; #define ATLEAST #endif +#ifndef __has_feature +#define __has_feature(x) 0 +#endif +#ifndef __has_extension +#define __has_extension __has_feature +#endif + +#if __has_feature(c_countof) +#define countof _Countof +#else +#define countof(a) (sizeof(a)/sizeof(*(a))) +#endif + /** Portability information **/ /* Determine OS, http://stackoverflow.com/questions/6649936 @@ -125,9 +138,9 @@ static const char *path_socketdir(void) GetProcAddress(GetModuleHandle("KERNEL32.DLL"), "GetTempPath2A"); DWORD rc; if (pGetTempPath2A != NULL) - rc = pGetTempPath2A(_countof(dir), dir); + rc = pGetTempPath2A(countof(dir), dir); else - rc = GetTempPathA(_countof(dir), dir); + rc = GetTempPathA(countof(dir), dir); if (rc == 0) failwith("could not get temporary path"); } @@ -534,7 +547,7 @@ static void compute_merlinpath(char merlin_path[ATLEAST PATHSZ], const char *arg strsz = strlen(merlin_path); // Append ocamlmerlin-server - if (strsz + sizeof(ocamlmerlin_server) + 8 > PATHSZ) + if (strsz + countof(ocamlmerlin_server) + 8 > PATHSZ) failwith("path is too long"); strcpy(merlin_path + strsz, ocamlmerlin_server); From 19fc132effa241f9b097ce6f56cd1a273eddbca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 12/25] ocamlmerlin.c: fearless booleans --- src/frontend/ocamlmerlin/ocamlmerlin.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 278d0598b..b2a8e7481 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -3,6 +3,7 @@ #include #include #include +#include #ifdef _WIN32 /* GetNamedPipeServerProcessId requires Windows Vista+ */ #undef _WIN32_WINNT @@ -276,7 +277,7 @@ static ssize_t prepare_args(unsigned char *buffer, size_t len, int argc, char ** #ifdef _WIN32 #define IPC_SOCKET_TYPE HANDLE -static HANDLE connect_socket(const char *socketname, int fail) +static HANDLE connect_socket(const char *socketname, bool fail) { HANDLE hPipe; hPipe = CreateFile(socketname, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); @@ -287,7 +288,7 @@ static HANDLE connect_socket(const char *socketname, int fail) #else #define IPC_SOCKET_TYPE int #define INVALID_HANDLE_VALUE -1 -static int connect_socket(const char *socketname, int fail) +static int connect_socket(const char *socketname, bool fail) { int sock = socket(PF_UNIX, SOCK_STREAM, 0); if (sock == -1) failwith_perror("socket"); @@ -429,12 +430,12 @@ static void start_server(const char *socketname, const char* ignored, const char static IPC_SOCKET_TYPE connect_and_serve(const char *socket_path, const char* event_path, const char *exec_path) { - IPC_SOCKET_TYPE sock = connect_socket(socket_path, 0); + IPC_SOCKET_TYPE sock = connect_socket(socket_path, false); if (sock == INVALID_HANDLE_VALUE) { start_server(socket_path, event_path, exec_path); - sock = connect_socket(socket_path, 1); + sock = connect_socket(socket_path, true); } if (sock == INVALID_HANDLE_VALUE) @@ -648,14 +649,14 @@ static void dumpinfo(void) static void unexpected_termination(int argc, char **argv) { - int sexp = 0; + bool sexp = false; int i; for (i = 1; i < argc - 1; ++i) { if (strcmp(argv[i], "-protocol") == 0 && strcmp(argv[i+1], "sexp") == 0) - sexp = 1; + sexp = true; } puts(sexp From 7475d0aa4e21c945113e65510758943cf6788959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 19 Nov 2025 08:35:18 +0100 Subject: [PATCH 13/25] ocamlmerlin.c: correctly report WinAPI errors `perror` can be used with functions from the CRT that set `errno`, otherwise `GetLastError` and `FormatMessage` have to be used instead. --- src/frontend/ocamlmerlin/ocamlmerlin.c | 41 ++++++++++++++++++-------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index b2a8e7481..d89f51bdb 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -98,6 +98,23 @@ typedef SSIZE_T ssize_t; static void dumpinfo(void); +#ifdef _WIN32 +static void failwith_formatmessage(const char *msg) +{ + char err[512]; + FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, /* message source */ + GetLastError(), /* error number */ + 0, /* default language */ + err, /* destination */ + countof(err), /* size of destination */ + NULL); /* no inserts */ + fprintf(stderr, "%s: %s\n", err, msg); + dumpinfo(); + exit(EXIT_FAILURE); +} +#endif + static void failwith_perror(const char *msg) { perror(msg); @@ -143,7 +160,7 @@ static const char *path_socketdir(void) else rc = GetTempPathA(countof(dir), dir); if (rc == 0) - failwith("could not get temporary path"); + failwith_formatmessage("GetTempPath"); } #else static const char *dir = NULL; @@ -162,9 +179,9 @@ static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, HANDLE fds { DWORD dwNumberOfBytesWritten; if (!WriteFile(hPipe, fds, 3 * sizeof(HANDLE), &dwNumberOfBytesWritten, NULL) || dwNumberOfBytesWritten != 3 * sizeof(HANDLE)) - failwith_perror("sendmsg"); + failwith_formatmessage("WriteFile/sendmsg"); if (!WriteFile(hPipe, buffer, len, &dwNumberOfBytesWritten, NULL) || dwNumberOfBytesWritten != len) - failwith_perror("send"); + failwith_formatmessage("WriteFile/send"); } #else @@ -282,7 +299,7 @@ static HANDLE connect_socket(const char *socketname, bool fail) HANDLE hPipe; hPipe = CreateFile(socketname, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); if (hPipe == INVALID_HANDLE_VALUE) - if (fail) failwith_perror("connect"); + if (fail) failwith_formatmessage("CreateFile/connect"); return hPipe; } #else @@ -338,11 +355,11 @@ static void start_server(const char *socketname, const char* eventname, const ch /* Note that DETACHED_PROCESS means that the process does not appear in Task Manager but the server can still be stopped with ocamlmerlin server stop-server */ if (!CreateProcess(exec_path, buf, NULL, NULL, FALSE, DETACHED_PROCESS, NULL, lpSystemDir, &si, &pi)) - failwith_perror("fork"); + failwith_formatmessage("CreateProcess/fork"); CloseHandle(pi.hProcess); CloseHandle(pi.hThread); if (WaitForSingleObject(hEvent, 5000) != WAIT_OBJECT_0) - failwith_perror("execlp"); + failwith_formatmessage("WaitForSingleObject/execlp"); } #else static void make_daemon(void) @@ -603,7 +620,7 @@ static void compute_socketname(char socketname[ATLEAST PATHSZ], char eventname[A LPSTR user_sid_string; HANDLE hFile = CreateFile(merlin_path, FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); if (hFile == INVALID_HANDLE_VALUE || !GetFileInformationByHandle(hFile, &info)) - failwith_perror("stat (cannot find ocamlmerlin binary)"); + failwith_formatmessage("CreateFile/stat (cannot find ocamlmerlin binary)"); CloseHandle(hFile); user_sid_string = retrieve_user_sid_string() ; @@ -697,17 +714,17 @@ int main(int argc, char **argv) #ifdef _WIN32 hProcess = GetCurrentProcess(); if (!GetNamedPipeServerProcessId(sock, &pid)) - failwith_perror("GetNamedPipeServerProcessId"); + failwith_formatmessage("GetNamedPipeServerProcessId"); hServerProcess = OpenProcess(PROCESS_DUP_HANDLE, FALSE, pid); if (hServerProcess == INVALID_HANDLE_VALUE) - failwith_perror("OpenProcess"); + failwith_formatmessage("OpenProcess"); if (!DuplicateHandle(hProcess, GetStdHandle(STD_INPUT_HANDLE), hServerProcess, &fds[0], 0, FALSE, DUPLICATE_SAME_ACCESS)) - failwith_perror("DuplicateHandle(stdin)"); + failwith_formatmessage("DuplicateHandle(stdin)"); if (!DuplicateHandle(hProcess, GetStdHandle(STD_OUTPUT_HANDLE), hServerProcess, &fds[1], 0, FALSE, DUPLICATE_SAME_ACCESS)) - failwith_perror("DuplicateHandle(stdout)"); + failwith_formatmessage("DuplicateHandle(stdout)"); CloseHandle(GetStdHandle(STD_OUTPUT_HANDLE)); if (!DuplicateHandle(hProcess, GetStdHandle(STD_ERROR_HANDLE), hServerProcess, &fds[2], 0, FALSE, DUPLICATE_SAME_ACCESS)) - failwith_perror("DuplicateHandle(stderr)"); + failwith_formatmessage("DuplicateHandle(stderr)"); #else int fds[3] = { STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO }; #endif From dee4d3a98935a72743b13e29c1c70c15ec074fe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 09:50:32 +0100 Subject: [PATCH 14/25] ocamlmerlin.c: use C99-conforming snprintf with mingw+msvcrt _snprintf doesn't null-terminate strings. MSVCRT doens't provide snprintf, but if __USE_MINGW_ANSI_STDIO is defined, then mingw-w64 will provide this function. There are environments where mingw-w64 is configured with MSVCRT, and others where it is configured with UCRT. It seems safer to use the compatibility shims. UCRT provides snprintf, and MSVC defaults to UCRT. --- src/frontend/ocamlmerlin/ocamlmerlin.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index d89f51bdb..a76b89ecc 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -1,3 +1,4 @@ +#define __USE_MINGW_ANSI_STDIO 1 #include #include #include @@ -23,9 +24,6 @@ #endif #ifdef _MSC_VER typedef SSIZE_T ssize_t; -#ifndef _UCRT -#define snprintf _snprintf -#endif #endif #else #include From a29431fbc51f16118958da7fa4111c4b8d47713a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 10:04:07 +0100 Subject: [PATCH 15/25] ocamlmerlin.c: mark some objects as const --- src/frontend/ocamlmerlin/ocamlmerlin.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index a76b89ecc..d5481ef00 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -173,7 +173,7 @@ static const char *path_socketdir(void) #ifdef _WIN32 /** Deal with Windows IPC **/ -static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, HANDLE fds[ATLEAST 3]) +static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, const HANDLE fds[ATLEAST 3]) { DWORD dwNumberOfBytesWritten; if (!WriteFile(hPipe, fds, 3 * sizeof(HANDLE), &dwNumberOfBytesWritten, NULL) || dwNumberOfBytesWritten != 3 * sizeof(HANDLE)) @@ -185,7 +185,7 @@ static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, HANDLE fds #else /** Deal with UNIX IPC **/ -static void ipc_send(int fd, unsigned char *buffer, size_t len, int fds[ATLEAST 3]) +static void ipc_send(int fd, unsigned char *buffer, size_t len, const int fds[ATLEAST 3]) { char msg_control[CMSG_SPACE(3 * sizeof(int))]; struct iovec iov = { .iov_base = buffer, .iov_len = len }; @@ -253,7 +253,7 @@ extern __declspec(dllimport) char **environ; extern char **environ; #endif -static ssize_t prepare_args(unsigned char *buffer, size_t len, int argc, char **argv) +static ssize_t prepare_args(unsigned char *buffer, size_t len, int argc, char * const *argv) { int i = 0; ssize_t j = 4; @@ -638,7 +638,7 @@ static void compute_socketname(char socketname[ATLEAST PATHSZ], char eventname[A } #else -static void compute_socketname(char socketname[ATLEAST SOCKSZ], struct stat *st) +static void compute_socketname(char socketname[ATLEAST SOCKSZ], const struct stat *st) { snprintf(socketname, SOCKSZ, "ocamlmerlin_%llu_%llu_%llu.socket", @@ -662,7 +662,7 @@ static void dumpinfo(void) "merlin path: %s\nsocket path: %s/%s\n", merlin_path, path_socketdir(), socketname); } -static void unexpected_termination(int argc, char **argv) +static void unexpected_termination(int argc, char * const *argv) { bool sexp = false; int i; From 1e493ebf30fd1a30ad2061937588c6ce8d56e1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 11:09:21 +0100 Subject: [PATCH 16/25] ocamlmerlin.c: fix sign-compare warnings --- src/frontend/ocamlmerlin/ocamlmerlin.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index d5481ef00..40318bf28 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -173,7 +173,7 @@ static const char *path_socketdir(void) #ifdef _WIN32 /** Deal with Windows IPC **/ -static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, const HANDLE fds[ATLEAST 3]) +static void ipc_send(HANDLE hPipe, unsigned char *buffer, ssize_t len, const HANDLE fds[ATLEAST 3]) { DWORD dwNumberOfBytesWritten; if (!WriteFile(hPipe, fds, 3 * sizeof(HANDLE), &dwNumberOfBytesWritten, NULL) || dwNumberOfBytesWritten != 3 * sizeof(HANDLE)) @@ -185,7 +185,7 @@ static void ipc_send(HANDLE hPipe, unsigned char *buffer, size_t len, const HAND #else /** Deal with UNIX IPC **/ -static void ipc_send(int fd, unsigned char *buffer, size_t len, const int fds[ATLEAST 3]) +static void ipc_send(int fd, unsigned char *buffer, ssize_t len, const int fds[ATLEAST 3]) { char msg_control[CMSG_SPACE(3 * sizeof(int))]; struct iovec iov = { .iov_base = buffer, .iov_len = len }; @@ -229,7 +229,7 @@ static void ipc_send(int fd, unsigned char *buffer, size_t len, const int fds[AT #define byte(x,n) ((unsigned)((x) >> (n * 8)) & 0xFF) -static void append_argument(unsigned char *buffer, size_t len, ssize_t *pos, const char *p) +static void append_argument(unsigned char *buffer, ssize_t len, ssize_t *pos, const char *p) { ssize_t j = *pos; while (*p && j < len) @@ -253,7 +253,7 @@ extern __declspec(dllimport) char **environ; extern char **environ; #endif -static ssize_t prepare_args(unsigned char *buffer, size_t len, int argc, char * const *argv) +static ssize_t prepare_args(unsigned char *buffer, ssize_t len, int argc, char * const *argv) { int i = 0; ssize_t j = 4; From 309b738793bee2da1a97fa5999a5bf0b361bd7a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 11:10:57 +0100 Subject: [PATCH 17/25] ocamlmerlin.c: avoid shadowing and mixing variables --- src/frontend/ocamlmerlin/ocamlmerlin.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 40318bf28..55345aa99 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -684,7 +684,6 @@ static void unexpected_termination(int argc, char * const *argv) int main(int argc, char **argv) { char result = 0; - int err = 0; struct stat st; #ifdef _WIN32 HANDLE fds[3]; @@ -730,11 +729,11 @@ int main(int argc, char **argv) #ifdef _WIN32 if (ReadFile(sock, &result, 1, &dwNumberOfBytesRead, NULL) && dwNumberOfBytesRead == 1) - err = 1; #else - NO_EINTR(err, read(sock, &result, 1)); + ssize_t read_; + NO_EINTR(read_, read(sock, &result, 1)); + if (read_ == 1) #endif - if (err == 1) exit(result); unexpected_termination(argc, argv); From eee30d7b1c713b4c1cc644960c3a8faff9d9b589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 26 Nov 2025 00:36:27 +0100 Subject: [PATCH 18/25] os_ipc_stub.c: use C99 for loop --- src/platform/os_ipc_stub.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/platform/os_ipc_stub.c b/src/platform/os_ipc_stub.c index 6f7384129..be61f6990 100644 --- a/src/platform/os_ipc_stub.c +++ b/src/platform/os_ipc_stub.c @@ -57,9 +57,7 @@ ml_merlin_set_environ(value venviron) if (environ) *environ = NULL; - size_t i, j; - - for (i = 0, j = 0; i < length; ++i) + for (size_t i = 0, j = 0; i < length; ++i) { if (buffer[i] == '\0') { @@ -142,20 +140,16 @@ static ssize_t recv_buffer(int fd, int fds[ATLEAST 3]) /* Check malformed packet */ if (nfds != 3 || recvd != target || buffer[recvd-1] != '\0') { - int i; - for (i = 0; i < nfds; ++i) + for (int i = 0; i < nfds; ++i) close(fds0[i]); return -1; } + for (int i = 0; i < 3; ++i) { - int i; - for (i = 0; i < 3; ++i) - { - fds[i] = fds0[i]; - if (fcntl(fds[i], F_SETFD, FD_CLOEXEC) == -1) - perror("fcntl"); - } + fds[i] = fds0[i]; + if (fcntl(fds[i], F_SETFD, FD_CLOEXEC) == -1) + perror("fcntl"); } return recvd; From 992016b5d801c71ed4133a2526658ea9f12eb533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 11:21:01 +0100 Subject: [PATCH 19/25] Ensure CMSG accesses are aligned --- src/frontend/ocamlmerlin/ocamlmerlin.c | 22 ++++++++++------------ src/platform/os_ipc_stub.c | 21 +++++++++++---------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index 55345aa99..ba9002260 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -187,24 +187,22 @@ static void ipc_send(HANDLE hPipe, unsigned char *buffer, ssize_t len, const HAN static void ipc_send(int fd, unsigned char *buffer, ssize_t len, const int fds[ATLEAST 3]) { - char msg_control[CMSG_SPACE(3 * sizeof(int))]; + union { + char buf[CMSG_SPACE(3 * sizeof(int))]; + struct cmsghdr align; + } u; struct iovec iov = { .iov_base = buffer, .iov_len = len }; - struct msghdr msg = { - .msg_iov = &iov, .msg_iovlen = 1, - .msg_controllen = CMSG_SPACE(3 * sizeof(int)), - }; - msg.msg_control = &msg_control; - memset(msg.msg_control, 0, msg.msg_controllen); + struct msghdr msg = { 0 }; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = &u.buf; + msg.msg_controllen = sizeof(u.buf); struct cmsghdr *cm = CMSG_FIRSTHDR(&msg); cm->cmsg_level = SOL_SOCKET; cm->cmsg_type = SCM_RIGHTS; cm->cmsg_len = CMSG_LEN(3 * sizeof(int)); - - int *fds0 = (int*)CMSG_DATA(cm); - fds0[0] = fds[0]; - fds0[1] = fds[1]; - fds0[2] = fds[2]; + memcpy(CMSG_DATA(cm), fds, 3 * sizeof(int)); ssize_t sent; NO_EINTR(sent, sendmsg(fd, &msg, 0)); diff --git a/src/platform/os_ipc_stub.c b/src/platform/os_ipc_stub.c index be61f6990..b90052134 100644 --- a/src/platform/os_ipc_stub.c +++ b/src/platform/os_ipc_stub.c @@ -81,14 +81,16 @@ static unsigned char buffer[BUFFER_SIZE]; static ssize_t recv_buffer(int fd, int fds[ATLEAST 3]) { - char msg_control[CMSG_SPACE(3 * sizeof(int))]; + union { + char buf[CMSG_SPACE(3 * sizeof(int))]; + struct cmsghdr align; + } u; struct iovec iov = { .iov_base = buffer, .iov_len = sizeof(buffer) }; - struct msghdr msg = { - .msg_iov = &iov, .msg_iovlen = 1, - .msg_controllen = CMSG_SPACE(3 * sizeof(int)), - }; - msg.msg_control = &msg_control; - memset(msg.msg_control, 0, msg.msg_controllen); + struct msghdr msg = { 0 }; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = &u.buf; + msg.msg_controllen = sizeof(u.buf); ssize_t recvd; NO_EINTR(recvd, recvmsg(fd, &msg, 0)); @@ -134,20 +136,19 @@ static ssize_t recv_buffer(int fd, int fds[ATLEAST 3]) perror("recvmsg"); return -1; } - int *fds0 = (int*)CMSG_DATA(cm); int nfds = (cm->cmsg_len - CMSG_LEN(0)) / sizeof(int); + memcpy(fds, CMSG_DATA(cm), nfds * sizeof(int)); /* Check malformed packet */ if (nfds != 3 || recvd != target || buffer[recvd-1] != '\0') { for (int i = 0; i < nfds; ++i) - close(fds0[i]); + close(fds[i]); return -1; } for (int i = 0; i < 3; ++i) { - fds[i] = fds0[i]; if (fcntl(fds[i], F_SETFD, FD_CLOEXEC) == -1) perror("fcntl"); } From 50af671ff5f4d28aaf91da073e5a8581010ba446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 17:48:17 +0100 Subject: [PATCH 20/25] ocamlmerlin.c: fix mingw-w64 format-truncation warnings --- src/frontend/ocamlmerlin/ocamlmerlin.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index ba9002260..b9cd22590 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -340,8 +340,11 @@ static void start_server(const char *socketname, const char* eventname, const ch PROCESS_INFORMATION pi; STARTUPINFO si; HANDLE hEvent = CreateEvent(NULL, FALSE, FALSE, eventname); + int len; - sprintf(buf, "%s server %s %s", exec_path, socketname, eventname); + len = snprintf(buf, countof(buf), "%s server %s %s", exec_path, socketname, eventname); + if (len < 0 || (unsigned)len > countof(buf) - 1) + failwith("arguments array was truncated"); ZeroMemory(&si, sizeof(si)); si.cb = sizeof(si); ZeroMemory(&pi, sizeof(pi)); @@ -623,14 +626,18 @@ static void compute_socketname(char socketname[ATLEAST PATHSZ], char eventname[A if (! user_sid_string) user_sid_string = LocalAlloc(LPTR, 1); + int len; // @@DRA Need to use Windows API functions to get meaningful values for st_dev and st_ino - snprintf(eventname, PATHSZ, - "ocamlmerlin_%s_%lx_%llx", - user_sid_string, - info.dwVolumeSerialNumber, - ((__int64)info.nFileIndexHigh) << 32 | ((__int64)info.nFileIndexLow)); - snprintf(socketname, PATHSZ, - "\\\\.\\pipe\\%s", eventname); + len = snprintf(eventname, PATHSZ, "ocamlmerlin_%s_%lx_%llx", + user_sid_string, + info.dwVolumeSerialNumber, + ((__int64)info.nFileIndexHigh) << 32 | ((__int64)info.nFileIndexLow)); + if (len < 0 || (unsigned) len > PATHSZ - 1) + failwith("event name was truncated"); + + len = snprintf(socketname, PATHSZ, "\\\\.\\pipe\\%s", eventname); + if (len < 0 || (unsigned) len > PATHSZ - 1) + failwith("socket name was truncated"); LocalFree(user_sid_string); } From 782afce3b0406a5b358633c55838bf9039a50e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 11:04:17 +0100 Subject: [PATCH 21/25] ocamlmerlin: use Dune to generate the C compiler invocation --- src/frontend/ocamlmerlin/dune | 49 +++++++++++++++++++------ src/frontend/ocamlmerlin/gen_ccflags.ml | 17 --------- 2 files changed, 38 insertions(+), 28 deletions(-) delete mode 100644 src/frontend/ocamlmerlin/gen_ccflags.ml diff --git a/src/frontend/ocamlmerlin/dune b/src/frontend/ocamlmerlin/dune index 8b17d252d..a3c44c7f6 100644 --- a/src/frontend/ocamlmerlin/dune +++ b/src/frontend/ocamlmerlin/dune @@ -14,7 +14,6 @@ -open Merlin_analysis -open Merlin_kernel -open Merlin_commands) - (modules (:standard \ gen_ccflags)) (libraries merlin-lib.config yojson merlin-lib.analysis merlin-lib.kernel merlin-lib.utils merlin-lib.os_ipc merlin-lib.ocaml_parsing merlin-lib.query_protocol merlin-lib.query_commands @@ -22,20 +21,48 @@ merlin-lib.commands unix str)) -(executable - (name gen_ccflags) - (modules gen_ccflags) - (libraries str)) +; Write one flag per line in the output file + +(rule + (enabled_if (= %{ocaml-config:ccomp_type} "cc")) + (action + (progn + (write-file ldlibs "") + (write-file outputexe "-o")))) + +(rule + (enabled_if (= %{ocaml-config:ccomp_type} "msvc")) + (action + (progn + (write-file ldlibs "advapi32.lib") + (write-file outputexe "-Fe")))) + +(rule + (enabled_if + (and + (= %{ocaml-config:ccomp_type} "cc") + (= %{profile} "dev"))) + (action + (write-file cflags "-Wextra\n-Werror"))) + +(rule + (enabled_if + (and + (= %{ocaml-config:ccomp_type} "msvc") + (= %{profile} "dev"))) + (action + (write-file cflags "-WX"))) (rule - (targets pre-flags post-flags) - (deps gen_ccflags.exe) - (action (run %{deps} "%{ocaml-config:ccomp_type}" %{targets}))) + (enabled_if + (<> %{profile} "dev")) + (action + (write-file cflags ""))) (rule - (targets ocamlmerlin.exe) - (deps (:c ocamlmerlin.c) pre-flags post-flags) - (action (run %{cc} "%{read-lines:pre-flags}%{targets}" %{c} %{read-lines:post-flags}))) + (target ocamlmerlin.exe) + (deps (:c ocamlmerlin.c)) + (action (run %{cc} %{read-strings:cflags} %{read:outputexe}%{target} %{c} %{read-strings:ldlibs}))) (install (package merlin) diff --git a/src/frontend/ocamlmerlin/gen_ccflags.ml b/src/frontend/ocamlmerlin/gen_ccflags.ml deleted file mode 100644 index 5bbf386b2..000000000 --- a/src/frontend/ocamlmerlin/gen_ccflags.ml +++ /dev/null @@ -1,17 +0,0 @@ -let ccomp_type = Sys.argv.(1) -let pre_flags_f = Sys.argv.(2) -let post_flags_f = Sys.argv.(3) - -let pre_flags, post_flags = - if Str.string_match (Str.regexp "msvc") ccomp_type 0 then - ("/Fe", "advapi32.lib") - else ("-o", "") - -let write_lines f s = - let oc = open_out f in - output_string oc s; - close_out oc - -let () = - write_lines pre_flags_f pre_flags; - write_lines post_flags_f post_flags From b36f7099247f3e9ebe86722a0a622311a90df978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 18:03:13 +0100 Subject: [PATCH 22/25] os_ipc_stub: fix unused variable warning --- src/platform/os_ipc_stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/os_ipc_stub.c b/src/platform/os_ipc_stub.c index b90052134..416ba208b 100644 --- a/src/platform/os_ipc_stub.c +++ b/src/platform/os_ipc_stub.c @@ -161,13 +161,13 @@ value ml_merlin_server_setup(value path, value strfd) { CAMLparam2(path, strfd); CAMLlocal2(payload, ret); - char *endptr = NULL; int fd; #ifdef _WIN32 fd = 0; ret = strfd; #else + char *endptr = NULL; fd = strtol(String_val(strfd), &endptr, 0); if (!endptr || *endptr != '\0') fd = -1; From 226400a1f54ca4e59b92f9e62cf5b0baac848fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 17:55:42 +0100 Subject: [PATCH 23/25] Don't run Make targets in parallel https://www.gnu.org/software/make/manual/html_node/Parallel-Disable.html --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 920fcf0f6..18dc627a0 100644 --- a/Makefile +++ b/Makefile @@ -26,3 +26,4 @@ bench: jq . merl-an_bench/bench.json .PHONY: all build dev clean test promote bench bench +.NOTPARALLEL: From 0528a3406cd41d3dc915939a762589f9939d0483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Tue, 25 Nov 2025 11:36:50 +0100 Subject: [PATCH 24/25] Update changes --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index a1b98ec3d..31a41585f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,8 @@ unreleased ========== + + merlin binary + - Fix a plethora of minor issues with the C code (#1998) + merlin library - Fix completion not working for inlined records labels (#1978, fixes #1977) - Perform buffer indexing only if the query requires it (#1990 and #1991) From 4e51ecbc428ee35d85edf81ce2d369919fd3e58c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Mon, 1 Dec 2025 15:37:02 +0000 Subject: [PATCH 25/25] ocamlmerlin.c: we're going to need a bigger buffer --- src/frontend/ocamlmerlin/ocamlmerlin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/ocamlmerlin/ocamlmerlin.c b/src/frontend/ocamlmerlin/ocamlmerlin.c index b9cd22590..8f7130cd9 100644 --- a/src/frontend/ocamlmerlin/ocamlmerlin.c +++ b/src/frontend/ocamlmerlin/ocamlmerlin.c @@ -336,7 +336,7 @@ static int connect_socket(const char *socketname, bool fail) #ifdef _WIN32 static void start_server(const char *socketname, const char* eventname, const char *exec_path) { - char buf[PATHSZ], lpSystemDir[PATHSZ]; + char buf[32767], lpSystemDir[PATHSZ]; PROCESS_INFORMATION pi; STARTUPINFO si; HANDLE hEvent = CreateEvent(NULL, FALSE, FALSE, eventname);