fix illegal instructions on Windows by MrScriptX · Pull Request #43 · jiacai2050/zig-curl

MrScriptX · 2026-02-17T08:21:36Z

Close #28 .

It was a curl bug that was fixed in later version. I updated curl to the latest version as of now (8.18.0)

gemini-code-assist · 2026-02-17T08:21:52Z

Summary of Changes

Hello @MrScriptX, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a reported Windows issue by upgrading the curl dependency to its latest version, 8.18.0. The upgrade necessitated significant changes to the libs/curl.zig build script to align with the new curl source directory structure and file organization. Additionally, a type definition in src/MultiPart.zig was updated to ensure compatibility with the new curl library's handling of file offsets.

Highlights

Dependency Update: Updated the curl dependency from version 8.8.0 to 8.18.0 to address a reported Windows-specific issue.
Build Configuration Adjustment: Adjusted the libs/curl.zig build configuration to align with the updated curl source file structure, including moving many files to a curlx subdirectory and adding/removing several source files.
Type Widening: Modified the seek function signature in src/MultiPart.zig to use c_longlong for the offset parameter, accommodating potentially larger file offsets.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

build.zig.zon
- Updated curl dependency version and hash.
libs/curl.zig
- Updated source file paths and included new files for the curl library build.
src/MultiPart.zig
- Changed offset parameter type in seek function.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request updates the curl dependency to version 8.18.0 to resolve a Windows-specific issue. The changes include updating the dependency definition, the list of C source files for curl, and a function signature to match the new API. I've found a duplicate source file in the build configuration which will likely cause a build failure. I've also suggested an improvement to make the source file list more maintainable. With the fix for the duplicate file, the PR should be good to go.

jiacai2050 · 2026-02-18T04:03:01Z

Please fix the CI errors.

MrScriptX · 2026-02-18T09:12:50Z

Ok, it should be fixed now. It was a CURL_OS macro that was expected, but not define in the curl.zig file.

jiacai2050 · 2026-02-18T09:44:16Z

The CI still failed

jiacai2050 · 2026-02-20T04:31:47Z

        "LICENSE",
    },
    .dependencies = .{
-        .curl = .{


Please update here directly

jiacai2050 · 2026-02-20T04:35:00Z

Could you also enable CI on windows ?

MrScriptX · 2026-02-20T14:34:06Z

Could you also enable CI on windows ?

There is a problem with the current CI as it is. zig fmt behave differently on Windows, which cause the Windows version to fail while the others don't. zig fmt should be split into another CI task and run only once (not on all platform).

jiacai2050 · 2026-02-20T16:29:52Z

zig fmt should be split into another CI task and run only once (not on all platform).

We could only run this on ubuntu.

MrScriptX · 2026-02-21T11:39:32Z

Ok, so what I did is create a new task for Ubuntu that runs make lint, and I removed lint as a dependency of test in the makefile so that they run independently.

jiacai2050 · 2026-02-22T07:02:12Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the vendored libcurl to version 8.18.0 to fix an issue with illegal instructions on Windows. The changes include updating the dependency in build.zig.zon, the documentation in README.org, and the vendored source files in libs/curl.zig. A new function getCurlOS has been added to correctly set platform-specific macros for curl, though it has some issues with memory management and string quoting. Additionally, a change in src/MultiPart.zig to support large file offsets has introduced a potential integer truncation bug. My review includes suggestions to address these issues.

Copilot

Pull request overview

Updates the vendored libcurl dependency and related build configuration to address the reported Windows “illegal instruction” crash (Issue #28), and expands CI to include Windows runs.

Changes:

Bump vendored curl from 8.8.0 to 8.18.0 and refresh the bundled curl source file list.
Adjust multipart seek callback signature for 64-bit offsets.
Add Windows to GitHub Actions CI matrix and introduce a make fix target.

Reviewed changes

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

Show a summary per file

File	Description
`src/MultiPart.zig`	Updates seek callback offset type to better match `curl_off_t` expectations.
`libs/curl.zig`	Updates curl compilation sources and adds a `CURL_OS` macro intended to align with newer curl builds.
`build.zig.zon`	Bumps curl dependency URL/hash to 8.18.0.
`README.org`	Updates documented vendored curl version.
`Makefile`	Adds `fix` target and adjusts `test` target behavior.
`.github/workflows/CI.yml`	Adds Windows to CI matrix and runs lint conditionally on Ubuntu.

Comments suppressed due to low confidence (1)

src/MultiPart.zig:85

seek now takes offset: c_longlong, but the SEEK_CUR path still casts source.offset to c_long before adding. On Windows c_long is 32-bit, so this can truncate large offsets; use a 64-bit type consistently for the addition/cast to match curl_off_t expectations.

                c.SEEK_SET => offset,
                c.SEEK_CUR => offset + @as(c_long, @intCast(source.offset)),
                else => return c.CURL_SEEKFUNC_FAIL,

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jiacai2050 · 2026-02-22T10:15:08Z

/gemini review

gemini-code-assist

Code Review

This pull request updates the vendored libcurl dependency to a newer version, aiming to fix an issue with illegal instructions on Windows. The changes involve updating the dependency URL and hash, adjusting the build scripts to accommodate the new version's source file structure, and adding logic to define the CURL_OS macro for different build targets. My review identified a critical issue with a broken dependency URL that will prevent the project from building. I've also found a high-severity bug related to potential data truncation, and a few medium-severity issues including a broken link in the documentation and opportunities to improve the new platform-detection logic for better maintainability.

gemini-code-assist · 2026-02-22T10:17:15Z

+            .url = "https://github.com/curl/curl/releases/download/curl-8_18_0/curl-8.18.0.tar.gz",
+            .hash = "N-V-__8AALp9QAGn6CCHZ6fK_FfMyGtG824LSHYHHasM3w-y",


The specified curl version 8.18.0 appears to be a typo, as this version does not exist. The download URL is broken, which will cause the build to fail. Based on the source file changes, the intended version is likely 8.9.0 or newer. Please correct the version, URL, and hash to point to a valid curl release.

For example, for version 8.9.0, the URL would be:
https://github.com/curl/curl/releases/download/curl-8_9_0/curl-8.9.0.tar.gz

You will need to update the hash accordingly.

gemini-code-assist · 2026-02-22T10:17:15Z

+        pub fn seek(user_data: ?*anyopaque, offset: c.curl_off_t, origin: c_int) callconv(.c) c_int {
+            var source: *DataWithOffset = @ptrCast(@alignCast(user_data orelse return c.CURL_SEEKFUNC_FAIL));


While changing offset's type to c.curl_off_t is correct, a related issue on line 84 could lead to bugs.

On line 84: c.SEEK_CUR => offset + @as(c_long, @intCast(source.offset))

The cast of source.offset (a usize) to c_long can cause truncation on platforms where usize is 64-bit and c_long is 32-bit (e.g., 64-bit Windows). This can lead to incorrect seek behavior for data larger than 4GB.

To prevent this, source.offset should be cast to c.curl_off_t to match the type of offset:

c.SEEK_CUR => offset + @as(c.curl_off_t, @intCast(source.offset)),

Since line 84 is not in the diff, I cannot provide a direct suggestion, but I strongly recommend applying this fix to prevent potential issues with large data slices.

gemini-code-assist · 2026-02-22T10:17:15Z

 lint:
 	zig fmt --check .

-test: lint
+test:


The test target no longer depends on the lint target. While this might speed up local testing, it also means that tests could be run on code that doesn't adhere to the project's formatting standards. It's generally a good practice to ensure code is linted before running tests to maintain code quality and consistency. Consider re-adding lint as a dependency for the test target.

gemini-code-assist · 2026-02-22T10:17:15Z

 | Library | Version |
 |---------+---------|
-| libcurl | [[https://github.com/curl/curl/tree/curl-8_8_0][8.8.0]]   |
+| libcurl | [[https://github.com/curl/curl/tree/curl-8_18_0][8.18.0]]   |


The curl version 8.18.0 appears to be a typo as this version does not exist in the official curl repository, and the link is broken. Based on the file changes in libs/curl.zig, the intended version seems to be 8.9.0 or newer. Please update this to the correct version to avoid confusing users.

| libcurl | [[https://github.com/curl/curl/tree/curl-8_9_0][8.9.0]] |

gemini-code-assist · 2026-02-22T10:17:15Z

+                .x86_64 => return "x86_64-pc-win32",
+                .thumb => return "thumbv7a-pc-win32",
+                .aarch64 => return "aarch64-pc-win32",
+                else => return "unknown-pc-win32",


For unhandled Windows architectures, returning null would be more consistent and informative. This would trigger the fallback logic in the caller, which constructs a system name like {arch}-pc-windows, providing more detail than the generic "unknown-pc-win32".

else => return null,

gemini-code-assist · 2026-02-22T10:17:15Z

+            switch (target.result.cpu.arch) {
+                .x86_64 => return "x86_64-pc-linux-gnu",
+                .aarch64 => return "aarch64-pc-linux-gnu",
+                else => return "Linux",


For unhandled Linux architectures, returning null would be more consistent and informative. This would trigger the fallback logic in the caller, which constructs a system name like {arch}-pc-linux, providing more detail than the generic "Linux".

else => return null,

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

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

Copilot · 2026-02-22T10:18:05Z

+            var source: *DataWithOffset = @ptrCast(@alignCast(user_data orelse return c.CURL_SEEKFUNC_FAIL));
            const new_pos = switch (origin) {
                c.SEEK_SET => offset,
                c.SEEK_CUR => offset + @as(c_long, @intCast(source.offset)),


The cast to c_long is inconsistent with the offset parameter type which is now c.curl_off_t. This should be changed to @as(c.curl_off_t, @intcast(source.offset)) to maintain type consistency and avoid potential overflow issues on platforms where curl_off_t and c_long have different sizes.

Suggested change

c.SEEK_CUR => offset + @as(c_long, @intCast(source.offset)),

c.SEEK_CUR => offset + @as(c.curl_off_t, @intCast(source.offset)),

- `libs/curl.zig`: Refactor `getCurlOS` to return an optional `[]const u8`. The logic for constructing the generic "arch-pc-os" system name for unknown platforms is now handled directly in the `create` function, rather than within `getCurlOS`. This makes `getCurlOS` responsible solely for identifying specific, known operating system names. - `src/MultiPart.zig`: Update the `offset` parameter in the `NonCopyingData.seek` callback to use `c.curl_off_t`. This improves type safety and aligns with the `curl` library's defined types for file offsets.

jiacai2050 · 2026-02-22T10:25:12Z

Thank you!

Dan Liebault added 2 commits February 16, 2026 18:11

update curl to 8.8 -> 8.18

57d6042

rm comments

4841794

gemini-code-assist Bot reviewed Feb 17, 2026

View reviewed changes

Comment thread libs/curl.zig

define CURL_OS as expected by makefile

4e98a87

update readme

6ddbcc6

Dan Liebault and others added 2 commits February 18, 2026 14:40

format error fixed

f038421

macro is text litteral and needs to be escaped

d1f6843

jiacai2050 requested changes Feb 20, 2026

View reviewed changes

MrScriptX added 5 commits February 20, 2026 15:14

update curl deps

58f3e6f

CURL_OS resolve is now a function

90ec2a0

zig fmt

077ec3a

enable windows ci

b14947c

disable windows ci

aaf0a45

jiacai2050 reviewed Feb 20, 2026

View reviewed changes

Comment thread build.zig.zon

MrScriptX added 3 commits February 21, 2026 12:27

set curl deps to lazy

3e4f91b

windows ci test

ff6a8c9

use makefile for linting

3a5c64f

jiacai2050 requested a review from Copilot February 22, 2026 07:02

Copilot started reviewing on behalf of jiacai2050 February 22, 2026 07:02 View session

gemini-code-assist Bot reviewed Feb 22, 2026

View reviewed changes

Comment thread libs/curl.zig Outdated

Comment thread src/MultiPart.zig Outdated

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Comment thread src/MultiPart.zig Outdated

Comment thread libs/curl.zig Outdated

Comment thread libs/curl.zig Outdated

Comment thread libs/curl.zig

Comment thread .github/workflows/CI.yml

Comment thread libs/curl.zig Outdated

jiacai2050 force-pushed the fix-windows branch from ea8b829 to 85ba827 Compare February 22, 2026 07:06

Refactor: Integrate linting into CI and add make fix command

7328a1f

jiacai2050 force-pushed the fix-windows branch from 85ba827 to 7328a1f Compare February 22, 2026 07:06

jiacai2050 and others added 2 commits February 22, 2026 17:57

Update MultiPart.zig

630c71a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update curl.zig

274036d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jiacai2050 requested a review from Copilot February 22, 2026 10:15

Copilot started reviewing on behalf of jiacai2050 February 22, 2026 10:15 View session

gemini-code-assist Bot reviewed Feb 22, 2026

View reviewed changes

Copilot AI reviewed Feb 22, 2026

View reviewed changes

jiacai2050 force-pushed the fix-windows branch from 5f87867 to 039f624 Compare February 22, 2026 10:21

jiacai2050 merged commit 1651b43 into jiacai2050:main Feb 22, 2026
4 checks passed

		.url = "https://github.com/curl/curl/releases/download/curl-8_18_0/curl-8.18.0.tar.gz",
		.hash = "N-V-__8AALp9QAGn6CCHZ6fK_FfMyGtG824LSHYHHasM3w-y",

		pub fn seek(user_data: ?*anyopaque, offset: c.curl_off_t, origin: c_int) callconv(.c) c_int {
		var source: *DataWithOffset = @ptrCast(@alignCast(user_data orelse return c.CURL_SEEKFUNC_FAIL));

	c.SEEK_CUR => offset + @as(c_long, @intCast(source.offset)),
	c.SEEK_CUR => offset + @as(c.curl_off_t, @intCast(source.offset)),

Conversation

MrScriptX commented Feb 17, 2026 • edited by jiacai2050 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot commented Feb 17, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

jiacai2050 commented Feb 18, 2026

Uh oh!

MrScriptX commented Feb 18, 2026

Uh oh!

jiacai2050 commented Feb 18, 2026

Uh oh!

jiacai2050 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jiacai2050 commented Feb 20, 2026

Uh oh!

MrScriptX commented Feb 20, 2026

Uh oh!

Uh oh!

jiacai2050 commented Feb 20, 2026

Uh oh!

MrScriptX commented Feb 21, 2026

Uh oh!

jiacai2050 commented Feb 22, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiacai2050 commented Feb 22, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

MrScriptX commented Feb 17, 2026 •

edited by jiacai2050

Loading