#69 support compile under Windows#723
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lidaobing
marked this pull request as draft
July 5, 2026 01:32
Closed
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #723 +/- ##
==========================================
- Coverage 51.88% 51.58% -0.30%
==========================================
Files 64 64
Lines 8714 8865 +151
==========================================
+ Hits 4521 4573 +52
- Misses 4193 4292 +99 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lidaobing
commented
Jul 6, 2026
Add a Windows implementation based on GetAdaptersAddresses and compute\nbroadcast addresses from each IPv4 unicast prefix. Keep the existing\nUnix ioctl path unchanged and add required Windows link libraries\n(iphlpapi, ws2_32) in meson for core/core-test targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances Windows build support for iptux (the C++/GTK LAN messenger) by introducing Windows-friendly build/link settings, consolidating networking platform includes, and replacing several POSIX-only helpers with GLib/GIO alternatives to improve cross-platform portability.
Changes:
- Add Windows-specific Meson dependencies/linking (e.g.,
ws2_32,iphlpapi) and a GitHub Actions Windows CI workflow update (MSYS2 CLANG64). - Centralize platform networking includes via
iptux_network.hand adjust code to use GLib/GIO APIs for IP parsing/formatting and file/path handling. - Introduce Windows path launching helper and gate/adjust tests and time APIs for Windows builds.
Reviewed changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/meson.build | Conditional crash utils / link args for Windows |
| src/main/iptux.cpp | Windows-safe localtime_* + disable crash handler |
| src/iptux/UiModelsTest.cpp | Use g_strdup; skip some tests on Windows |
| src/iptux/UiModels.cpp | Update message source enum name |
| src/iptux/UIHelperWindows.h | Declare Windows “open path” helper |
| src/iptux/UIHelperWindows.cpp | Implement Windows path launcher (ShellExecuteW) |
| src/iptux/UiHelperTest.cpp | Skip TZ/image tests on Windows |
| src/iptux/UiHelper.h | Use iptux_network.h; add iptux_open_path |
| src/iptux/UiHelper.cpp | Implement iptux_open_path + Windows branch; localtime_* |
| src/iptux/UiCoreThread.h | Remove non-portable network include |
| src/iptux/UiCoreThread.cpp | Remove non-portable socket include |
| src/iptux/RevisePal.cpp | Cast for ntohl formatting portability |
| src/iptux/meson.build | Add Windows deps and build UiHelperWindows.cpp |
| src/iptux/MainWindow.h | Undef Windows CreateWindow macro collision |
| src/iptux/MainWindow.cpp | Add Windows strcasestr compat + localtime_* |
| src/iptux/LogSystem.cpp | Add logging, error checks, GLib path building |
| src/iptux/DialogPeer.cpp | Use g_build_filename for received file paths |
| src/iptux/DataSettings.h | Add missing <string> include |
| src/iptux/DataSettings.cpp | Replace getline/inet_pton flow with portable parsing helpers |
| src/iptux/callback.cpp | Use nullptr for GTK dialog sentinel |
| src/iptux/Application.cpp | Avoid SIGPIPE handling on Windows; open log paths |
| src/iptux-utils/UtilsTest.cpp | Skip dupPath test on Windows |
| src/iptux-utils/utils.h | Introduce iptux_network.h, add GSocket read/write overloads, add is_ipv4 |
| src/iptux-utils/utils.cpp | Use GLib networking helpers; add GSocket read/write; portable dir iteration |
| src/iptux-utils/TestHelper.cpp | Read test data as binary |
| src/iptux-utils/output.h | Qualify logging macros with ::iptux:: |
| src/iptux-utils/output.cpp | Switch timestamp formatting to GDateTime |
| src/iptux-utils/meson.build | Add Windows libs to utils deps |
| src/iptux-utils/Exception.cpp | Inline getErrorCode() in header |
| src/iptux-core/ProgramData.cpp | Use g_strdup for filepath duplication |
| src/iptux-core/ModelsTest.cpp | Use new IP helpers in tests |
| src/iptux-core/Models.cpp | Remove non-portable network include |
| src/iptux-core/meson.build | Add Windows libs (ws2_32, iphlpapi) to core deps |
| src/iptux-core/internal/TcpData.cpp | Use O_BINARY; use this->socket consistently |
| src/iptux-core/internal/support.cpp | Add Windows broadcast discovery path; adjust socket reuse helper |
| src/iptux-core/internal/SendFileData.h | Switch from fd int to GSocket* |
| src/iptux-core/internal/SendFileData.cpp | Use GSocket*, O_BINARY, GLib basename/IP formatting |
| src/iptux-core/internal/SendFile.h | Switch send APIs to GSocket* |
| src/iptux-core/internal/SendFile.cpp | Adapt send pipeline to GSocket* + GLib basename |
| src/iptux-core/internal/RecvFileData.h | Add GOutputStream* receive path |
| src/iptux-core/internal/RecvFileData.cpp | Use GIO stream writing for Windows-friendly receiving |
| src/iptux-core/internal/iptux_network.h | New cross-platform networking include hub |
| src/iptux-core/internal/Command.cpp | Use O_BINARY; update enum usage; sendto cast tweak |
| src/iptux-core/internal/AnalogFS.cpp | Use GLib path building/current dir for portability |
| src/iptux-core/CoreThread.cpp | Remove non-portable socket include |
| src/api/iptux-core/TransFileModel.h | Add <cstdint> include |
| src/api/iptux-core/Models.h | Windows networking include + enum rename |
| src/api/iptux-core/Exception.h | Inline getErrorCode() |
| src/api/iptux-core/CoreThread.h | Remove non-portable network include |
| meson.build | Add warning suppressions helpful for clang/Windows toolchains |
| .github/workflows/windows.yml | Update Windows CI (checkout v4, MSYS2 CLANG64, extra checks) |
| .gitattributes | Enforce LF endings and mark binaries/testdata |
Comments suppressed due to low confidence (1)
src/iptux-core/internal/RecvFileData.cpp:521
- g_output_stream_write_all() returns gboolean (TRUE/FALSE), not -1. Comparing the return value to -1 will never detect failures, so receive-to-file may silently ignore write errors and report success incorrectly.
Comment on lines
+7
to
+28
| void iptux_launch_file_windows(const char* utf8_path) { | ||
| int len = MultiByteToWideChar(CP_UTF8, 0, utf8_path, -1, NULL, 0); | ||
| wchar_t* w_path = (wchar_t*)malloc(len * sizeof(wchar_t)); | ||
| MultiByteToWideChar(CP_UTF8, 0, utf8_path, -1, w_path, len); | ||
|
|
||
| HINSTANCE result = ShellExecuteW( | ||
| NULL, // 父窗口句柄 | ||
| L"open", // 操作类型：打开 | ||
| w_path, // 文件路径 | ||
| NULL, // 命令行参数 | ||
| NULL, // 工作目录 | ||
| SW_SHOWNORMAL // 显示方式 | ||
| ); | ||
|
|
||
| // 3. 释放内存 | ||
| free(w_path); | ||
|
|
||
| // 检查是否成功 | ||
| if ((INT_PTR)result <= 32) { | ||
| LOG_ERROR("Failed to open path: %s, error code: %jd", utf8_path, (intmax_t)(INT_PTR)result); | ||
| } | ||
| } |
lidaobing
commented
Jul 9, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
lidaobing
marked this pull request as ready for review
July 14, 2026 05:13
Reviewer's GuideEnable building and running iptux on Windows by introducing a cross‑platform networking abstraction, replacing POSIX‑only APIs with GLib/GIO and Windows‑aware code paths, and tightening file/log handling and tests while updating the Windows CI workflow. Sequence diagram for TCP file data request using GSocketsequenceDiagram
participant TcpHandlerThread as TcpHandlerThreadFunc
participant TcpData as TcpData
participant CoreThread as CoreThread
participant SendFile as SendFile
participant SendFileData as SendFileData
TcpHandlerThread->>TcpData: TcpDataEntry(coreThread, clientSocket, error)
TcpData->>TcpData: DispatchTcpData(error)
TcpData->>CoreThread: GetPal(gaddr)
CoreThread-->>TcpData: PalInfo::Ptr
TcpData->>TcpData: read_ipmsg_prefix(socket, buf, MAX_SOCKLEN)
TcpData->>SendFile: RequestDataEntry(coreThread, socket, fileattr, attach)
SendFile->>SendFileData: new SendFileData(coreThread, socket, file)
SendFileData->>SendFileData: SendFileDataEntry()
SendFileData->>SendFileData: SendRegularFile()
SendFileData->>SendFileData: SendData(fd, filesize)
SendFileData->>socket: xwrite(socket, buf, size)
Sequence diagram for receiving regular file with GSocket and GOutputStreamsequenceDiagram
participant RecvFileData as RecvFileData
participant Command as Command
participant GSocket as GSocket
participant GFileOutputStream as GFileOutputStream
RecvFileData->>GSocket: g_socket_new(...)
RecvFileData->>Command: SendAskData(GSocket, key, packetn, fileid, 0)
Command-->>RecvFileData: success/fail
RecvFileData->>RecvFileData: assert_filename_inexist(file->filepath)
RecvFileData->>GFileOutputStream: g_file_replace(gfile,..., error)
RecvFileData->>RecvFileData: RecvData(GSocket, GOutputStream, filesize, 0)
RecvFileData->>GSocket: g_socket_receive(sock, buf, size,...)
RecvFileData->>GFileOutputStream: g_output_stream_write_all(os, buf, size,...)
RecvFileData->>GFileOutputStream: g_output_stream_close(os,...)
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 6 issues, and left some high level feedback:
- In CoreThread's tcpHandlerThreadFunc,
resis used after the try/catch block but is never initialized if TcpDataEntry throws before assigning it; consider initializingresto a known value and only checking it when the call actually returns. - TcpData::DispatchTcpData assumes the GError** passed in is non-null and calls g_set_error/g_set_error_literal unconditionally; it would be safer either to require and assert a non-null error pointer or guard these calls when error is nullptr to avoid undefined behavior.
- RecvFileData::RecvRegularFile now computes a
fullpathvia assert_filename_inexist and writes to that path, but utime is still called withfile->filepath; updating timestamps onfullpathinstead would avoid mismatches when the destination filename is altered.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In CoreThread's tcpHandlerThreadFunc, `res` is used after the try/catch block but is never initialized if TcpDataEntry throws before assigning it; consider initializing `res` to a known value and only checking it when the call actually returns.
- TcpData::DispatchTcpData assumes the GError** passed in is non-null and calls g_set_error/g_set_error_literal unconditionally; it would be safer either to require and assert a non-null error pointer or guard these calls when error is nullptr to avoid undefined behavior.
- RecvFileData::RecvRegularFile now computes a `fullpath` via assert_filename_inexist and writes to that path, but utime is still called with `file->filepath`; updating timestamps on `fullpath` instead would avoid mismatches when the destination filename is altered.
## Individual Comments
### Comment 1
<location path="src/iptux-core/internal/RecvFileData.cpp" line_range="462-463" />
<code_context>
return finishsize;
+ }
+
+ gsize bytes_written = 0;
+ if (g_output_stream_write_all(os, buf, size, &bytes_written, nullptr,
+ &error) == -1) {
+ LOG_WARN("g_output_stream_write_all failed: %s", error->message);
</code_context>
<issue_to_address>
**issue (bug_risk):** Incorrect check of g_output_stream_write_all() return value
`g_output_stream_write_all()` returns a `gboolean`, so the `== -1` check will never trigger and write errors will be ignored, leaving `error` potentially unset. This should instead check for `FALSE` and only use `error` when non-null. Also verify the intended return value semantics on short writes; `finishsize + bytes_written` may not match what callers expect.
</issue_to_address>
### Comment 2
<location path="src/iptux-utils/utils.cpp" line_range="711-720" />
<code_context>
return offset;
}
+ssize_t xread(GSocket* sock, void* buf, size_t count) {
+ size_t offset;
+ ssize_t size;
+
+ size = -1;
+ offset = 0;
+ while ((offset != count) && (size != 0)) {
+ if ((size = g_socket_receive(sock, (gchar*)buf + offset, count - offset,
+ NULL, NULL)) == -1) {
+ if (errno == EINTR)
+ continue;
+ return -1;
</code_context>
<issue_to_address>
**issue (bug_risk):** Using errno with g_socket_receive() is unreliable; prefer GError
These GSocket-based helpers assume g_socket_receive() sets errno and treat EINTR specially, but GLib reports errors via GError, not errno. This can lead to incorrect handling of errors/interrupts. Please use the GError** parameter, inspect the error domain/code for interruptions (or rely on GLib’s built-in handling), and avoid errno-based logic with GSocket.
</issue_to_address>
### Comment 3
<location path="src/iptux/DataSettings.cpp" line_range="1113" />
<code_context>
+ if (*(buffer + strspn(buffer, "\t\x20")) == '#')
continue;
- switch (sscanf(lineptr, "%s - %s //%s", buf[0], buf[1], buf[2])) {
+ switch (sscanf(buffer, "%1023s - %1023s //%1023s", buf[0], buf[1], buf[2])) {
case 3:
- if (inet_pton(AF_INET, buf[0], &ipv4) <= 0 ||
</code_context>
<issue_to_address>
**issue (bug_risk):** sscanf width specifiers should be tied to MAX_BUFLEN, not hard-coded 1023
`buf` is `char buf[3][MAX_BUFLEN]`, but the format string uses `%1023s`. If `MAX_BUFLEN` is reduced below 1024, this can overflow the buffer. Please derive the width from `MAX_BUFLEN` (e.g., via `G_STRINGIFY(MAX_BUFLEN-1)` or a compile-time constant) so it always matches the buffer size.
</issue_to_address>
### Comment 4
<location path="src/iptux-core/internal/AnalogFS.cpp" line_range="55-64" />
<code_context>
* @return 成功与否
*/
int AnalogFS::chdir(const char* dir) {
- size_t len;
- char* ptr;
-
- if (strcmp(dir, ".") == 0)
- return 0;
+ char* new_path;
- if (*dir != '/') {
- if (strcmp(dir, "..") == 0) {
- ptr = strrchr(path, '/');
- if (ptr != path)
- *ptr = '\0';
- } else {
- len = strlen(path);
- ptr = (char*)(*(path + 1) != '\0' ? "/" : "");
- snprintf(path + len, MAX_PATHLEN - len, "%s%s", ptr, dir);
- }
- } else
- snprintf(path, MAX_PATHLEN, "%s", dir);
+ if (g_path_is_absolute(dir)) {
+ new_path = g_strdup(dir);
+ } else {
+ new_path = g_build_filename(this->path, dir, NULL);
+ }
+ if (!new_path) {
+ LOG_WARN("Failed to build new path from \"%s\" and \"%s\"", this->path,
+ dir);
+ return -1;
+ }
+ if (strlen(new_path) >= MAX_PATHLEN) {
+ LOG_WARN("New path \"%s\" exceeds maximum length of %d", new_path,
+ MAX_PATHLEN);
+ g_free(new_path);
+ return -1;
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** New chdir/mergepath implementation no longer handles ".." semantics
Previously, AnalogFS::chdir/mergepath explicitly processed "." and ".." to update `path` and move up directories. The new code delegates to `g_build_filename()`, which leaves `..` segments in the string (e.g. `path/..`) instead of normalizing them. Any callers expecting filesystem-like handling of `.`/`..` may now see different traversal behavior. If this change is not deliberate, consider normalizing the resulting path (e.g., via GFile or manual `.`/`..` collapsing) before storing it.
</issue_to_address>
### Comment 5
<location path="src/iptux-core/internal/TcpData.cpp" line_range="60" />
<code_context>
static gpointer tcpHandlerThreadFunc(gpointer data) {
TcpHandlerData* handlerData = static_cast<TcpHandlerData*>(data);
+ GError* error = nullptr;
+ bool res;
+
try {
</code_context>
<issue_to_address>
**issue (bug_risk):** Possible use of uninitialized `res` when an exception is thrown early
In `tcpHandlerThreadFunc`, `bool res` is only set inside the `try` block. If `TcpDataEntry` throws before `res` is assigned, the later `if (!res)` will read an uninitialized value, causing undefined behavior. Initialize `res` to `false` before the `try` block to ensure safe and consistent error handling.
</issue_to_address>
### Comment 6
<location path="src/iptux-utils/utils.cpp" line_range="646" />
<code_context>
return offset;
}
+ssize_t xwrite(GSocket* sock, const void* buf, size_t count) {
+ size_t offset;
+ ssize_t size;
</code_context>
<issue_to_address>
**suggestion:** Logging errno for GSocket write errors may be misleading
In this GSocket-based xwrite(), the error handling currently logs `strerror(errno)`, but g_socket_send() reports failures via GError, so errno may not reflect the actual issue. To keep accurate diagnostics, use a GError** with g_socket_send(), log the GError message, and avoid relying on errno in this path.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+462
to
+463
| gsize bytes_written = 0; | ||
| if (g_output_stream_write_all(os, buf, size, &bytes_written, nullptr, |
There was a problem hiding this comment.
issue (bug_risk): Incorrect check of g_output_stream_write_all() return value
g_output_stream_write_all() returns a gboolean, so the == -1 check will never trigger and write errors will be ignored, leaving error potentially unset. This should instead check for FALSE and only use error when non-null. Also verify the intended return value semantics on short writes; finishsize + bytes_written may not match what callers expect.
Comment on lines
+711
to
+720
| ssize_t xread(GSocket* sock, void* buf, size_t count) { | ||
| size_t offset; | ||
| ssize_t size; | ||
|
|
||
| size = -1; | ||
| offset = 0; | ||
| while ((offset != count) && (size != 0)) { | ||
| if ((size = g_socket_receive(sock, (gchar*)buf + offset, count - offset, | ||
| NULL, NULL)) == -1) { | ||
| if (errno == EINTR) |
There was a problem hiding this comment.
issue (bug_risk): Using errno with g_socket_receive() is unreliable; prefer GError
These GSocket-based helpers assume g_socket_receive() sets errno and treat EINTR specially, but GLib reports errors via GError, not errno. This can lead to incorrect handling of errors/interrupts. Please use the GError** parameter, inspect the error domain/code for interruptions (or rely on GLib’s built-in handling), and avoid errno-based logic with GSocket.
| if (*(buffer + strspn(buffer, "\t\x20")) == '#') | ||
| continue; | ||
| switch (sscanf(lineptr, "%s - %s //%s", buf[0], buf[1], buf[2])) { | ||
| switch (sscanf(buffer, "%1023s - %1023s //%1023s", buf[0], buf[1], buf[2])) { |
There was a problem hiding this comment.
issue (bug_risk): sscanf width specifiers should be tied to MAX_BUFLEN, not hard-coded 1023
buf is char buf[3][MAX_BUFLEN], but the format string uses %1023s. If MAX_BUFLEN is reduced below 1024, this can overflow the buffer. Please derive the width from MAX_BUFLEN (e.g., via G_STRINGIFY(MAX_BUFLEN-1) or a compile-time constant) so it always matches the buffer size.
Comment on lines
-55
to
-64
| return 0; | ||
| char* new_path; | ||
|
|
||
| if (*dir != '/') { | ||
| if (strcmp(dir, "..") == 0) { | ||
| ptr = strrchr(path, '/'); | ||
| if (ptr != path) | ||
| *ptr = '\0'; | ||
| } else { | ||
| len = strlen(path); | ||
| ptr = (char*)(*(path + 1) != '\0' ? "/" : ""); |
There was a problem hiding this comment.
question (bug_risk): New chdir/mergepath implementation no longer handles ".." semantics
Previously, AnalogFS::chdir/mergepath explicitly processed "." and ".." to update path and move up directories. The new code delegates to g_build_filename(), which leaves .. segments in the string (e.g. path/..) instead of normalizing them. Any callers expecting filesystem-like handling of ./.. may now see different traversal behavior. If this change is not deliberate, consider normalizing the resulting path (e.g., via GFile or manual ./.. collapsing) before storing it.
| tdata.socket = socket; | ||
| tdata.sock = g_socket_get_fd(socket); | ||
| tdata.DispatchTcpData(); | ||
| return tdata.DispatchTcpData(error); |
There was a problem hiding this comment.
issue (bug_risk): Possible use of uninitialized res when an exception is thrown early
In tcpHandlerThreadFunc, bool res is only set inside the try block. If TcpDataEntry throws before res is assigned, the later if (!res) will read an uninitialized value, causing undefined behavior. Initialize res to false before the try block to ensure safe and consistent error handling.
| return offset; | ||
| } | ||
|
|
||
| ssize_t xwrite(GSocket* sock, const void* buf, size_t count) { |
There was a problem hiding this comment.
suggestion: Logging errno for GSocket write errors may be misleading
In this GSocket-based xwrite(), the error handling currently logs strerror(errno), but g_socket_send() reports failures via GError, so errno may not reflect the actual issue. To keep accurate diagnostics, use a GError** with g_socket_send(), log the GError message, and avoid relying on errno in this path.
|
❌ The last analysis has failed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/close #69
Summary by Sourcery
Modernize time formatting and networking includes while aligning string allocation with GLib utilities.
Bug Fixes:
Enhancements:
Tests:
Summary by Sourcery
Add Windows support for building and running iptux, including network, file transfer, logging, and UI behavior adaptations, while tightening IP handling and error reporting across platforms.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Tests: