fix(c): port example_common.h to callback API + add port-forwarding example and VM-free option test#1004
Open
xxxzl114514 wants to merge 1 commit into
Conversation
…le + VM-free option test
📦 BoxLite review — 3 issues ·
|
📝 WalkthroughWalkthroughChangesC SDK examples and validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Main as C example
participant Runtime as BoxLite runtime
participant Options as CBoxliteOptions
participant GuestBox as Guest box
participant Exec as Execution callbacks
Main->>Runtime: create runtime
Main->>Options: configure port and volume
Main->>Runtime: create guest box
Runtime-->>Main: return handle callback
Main->>GuestBox: execute hostname
GuestBox->>Exec: stream output and exit code
Main->>Runtime: free resources
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![boxlite-agent[bot]](https://avatars.githubusercontent.com/in/4244458?s=60&v=4){kind=small}
Comment on lines
+56
to
+58
| BoxliteErrorCode code = | ||
| boxlite_create_box(runtime, opts, on_box_created, &ctx, &error); | ||
| boxlite_options_free(opts); |
There was a problem hiding this comment.
🛑 opts double-freed after boxlite_create_box consumes it
boxlite_create_box does Box::from_raw(opts) synchronously in box_handle.rs:145 whenever cb is non-null (always true here), so opts is already Rust-owned when Ok is returned; the next line calls boxlite_options_free(opts) again — use-after-free/double-free on every call, confirmed by reading box_handle.rs::create_box and options.rs::options_free, and by test_null_callback.c's own comment 'opts is only consumed by boxlite_create_box on success — free here' (this path is exactly that success case, so must NOT free).
Suggested change
| BoxliteErrorCode code = | |
| boxlite_create_box(runtime, opts, on_box_created, &ctx, &error); | |
| boxlite_options_free(opts); | |
| CreateBoxCtx ctx = {0}; | |
| CBoxliteError error = {0}; | |
| BoxliteErrorCode code = | |
| boxlite_create_box(runtime, opts, on_box_created, &ctx, &error); |
Comment on lines
+52
to
+65
| static CBoxHandle *create_box_with_options_or_exit(CBoxliteRuntime *runtime, | ||
| CBoxliteOptions *opts) { | ||
| CreateBoxCtx ctx = {0}; | ||
| CBoxliteError error = {0}; | ||
| BoxliteErrorCode code = boxlite_create_box(runtime, opts, &box, &error); | ||
| if (code != Ok) { | ||
| BoxliteErrorCode code = | ||
| boxlite_create_box(runtime, opts, on_box_created, &ctx, &error); | ||
| boxlite_options_free(opts); | ||
| if (code != Ok || ctx.box == NULL) { | ||
| print_error("box creation", &error); | ||
| boxlite_error_free(&error); | ||
| return NULL; | ||
| } | ||
| return box; | ||
| return ctx.box; | ||
| } |
There was a problem hiding this comment.
🛑 callbacks never fire: no boxlite_runtime_drain call anywhere
Callbacks are dispatched exclusively inside boxlite_runtime_drain's dispatch_event loop (runtime.rs:404-461); boxlite_create_box itself only enqueues a RuntimeEvent and returns Ok immediately. No file under examples/c/ calls boxlite_runtime_drain, so on_box_created/on_stdout_chunk/on_stderr_chunk/on_wait_done never execute — ctx.box stays NULL forever, so create_box_with_options_or_exit always prints a spurious 'box creation failed (code 0)' and returns NULL, breaking every ported example including the new 06_port_forwarding.c.
Comment on lines
+46
to
+52
| - �?Structured error handling (error codes + messages) | ||
| - �?OCI container images | ||
| - �?Hardware-accelerated VMs (KVM/Hypervisor.framework) | ||
| - �?Command execution with streaming output | ||
| - �?Box lifecycle management | ||
| - �?Performance metrics | ||
| - �?Multi-box management |
There was a problem hiding this comment.
Dozens of check/cross/arrow characters were replaced with a literal '<0xEF><0xBF><0xBD>?' replacement-character sequence across the whole file (headings, tables, migration guide), apparently unintended encoding corruption rather than a real content edit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/c/06_port_forwarding.c`:
- Around line 58-59: Update the execute_and_wait failure branch in the
port-forwarding example to print the error, free the error, perform required
cleanup, and return a non-zero status immediately. Ensure the “[exit %d]”
success path is reached only when execution succeeds.
In `@examples/c/example_common.h`:
- Around line 80-87: Update the forward function to process data in multiple
chunks instead of truncating input beyond the fixed buffer size. Loop until all
len bytes are forwarded, copying each segment into buf, null-terminating it, and
invoking c->cb with the original stream and user-data context.
In `@sdks/c/README.md`:
- Around line 46-52: Replace all corrupted `�?` glyphs throughout
README.md—including the feature list, memory-management mappings, threading
note, platform table, migration headings, and architecture diagram—with valid
UTF-8 symbols or plain ASCII equivalents, preserving the surrounding content and
formatting.
In `@sdks/c/tests/test_options_port.c`:
- Around line 20-64: Remove the err resets, FREE_IF_ERR macro, and related calls
from the boxlite_options_add_port assertions because that API does not populate
a CBoxliteError; retain error cleanup only for boxlite_options_new where
applicable. Rename the status variable c to code throughout main, including its
declaration, assignments, checks, and messages, while preserving the existing
result assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a631587b-55ad-44cc-8af5-bfe4d336ef58
📒 Files selected for processing (5)
examples/c/06_port_forwarding.cexamples/c/example_common.hsdks/c/README.mdsdks/c/tests/CMakeLists.txtsdks/c/tests/test_options_port.c
Comment on lines
+58
to
+59
| if (code != Ok) { print_error("hostname", &error); boxlite_error_free(&error); } | ||
| printf("\n[exit %d]\n", exit_code); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exit with an error status on execution failure.
If execute_and_wait fails, the example logs the error but proceeds to incorrectly print [exit 0] and eventually returns a 0 exit code. The example should clean up and return a non-zero exit code to accurately reflect the failure.
🛠️ Proposed fix
- if (code != Ok) { print_error("hostname", &error); boxlite_error_free(&error); }
- printf("\n[exit %d]\n", exit_code);
+ if (code != Ok) {
+ print_error("hostname", &error);
+ boxlite_error_free(&error);
+ boxlite_free_string(box_id);
+ boxlite_runtime_free(runtime);
+ return 1;
+ }
+ printf("\n[exit %d]\n", exit_code);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (code != Ok) { print_error("hostname", &error); boxlite_error_free(&error); } | |
| printf("\n[exit %d]\n", exit_code); | |
| if (code != Ok) { | |
| print_error("hostname", &error); | |
| boxlite_error_free(&error); | |
| boxlite_free_string(box_id); | |
| boxlite_runtime_free(runtime); | |
| return 1; | |
| } | |
| printf("\n[exit %d]\n", exit_code); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/c/06_port_forwarding.c` around lines 58 - 59, Update the
execute_and_wait failure branch in the port-forwarding example to print the
error, free the error, perform required cleanup, and return a non-zero status
immediately. Ensure the “[exit %d]” success path is reached only when execution
succeeds.
Comment on lines
+80
to
+87
| static void forward(const uint8_t *data, size_t len, int is_stderr, ExecCtx *c) { | ||
| if (c->cb == NULL || len == 0) return; | ||
| char buf[4096]; | ||
| size_t n = len < sizeof(buf) - 1 ? len : sizeof(buf) - 1; | ||
| memcpy(buf, data, n); | ||
| buf[n] = '\0'; | ||
| c->cb(buf, is_stderr, c->user_data); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Prevent data truncation for large stream chunks.
If the incoming data chunk is larger than 4095 bytes, the remaining bytes are silently dropped. Loop over the incoming data to ensure the entire chunk is forwarded.
🐛 Proposed fix to loop over the buffer
static void forward(const uint8_t *data, size_t len, int is_stderr, ExecCtx *c) {
if (c->cb == NULL || len == 0) return;
char buf[4096];
- size_t n = len < sizeof(buf) - 1 ? len : sizeof(buf) - 1;
- memcpy(buf, data, n);
- buf[n] = '\0';
- c->cb(buf, is_stderr, c->user_data);
+ size_t offset = 0;
+ while (offset < len) {
+ size_t chunk = (len - offset) < (sizeof(buf) - 1) ? (len - offset) : (sizeof(buf) - 1);
+ memcpy(buf, data + offset, chunk);
+ buf[chunk] = '\0';
+ c->cb(buf, is_stderr, c->user_data);
+ offset += chunk;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static void forward(const uint8_t *data, size_t len, int is_stderr, ExecCtx *c) { | |
| if (c->cb == NULL || len == 0) return; | |
| char buf[4096]; | |
| size_t n = len < sizeof(buf) - 1 ? len : sizeof(buf) - 1; | |
| memcpy(buf, data, n); | |
| buf[n] = '\0'; | |
| c->cb(buf, is_stderr, c->user_data); | |
| } | |
| static void forward(const uint8_t *data, size_t len, int is_stderr, ExecCtx *c) { | |
| if (c->cb == NULL || len == 0) return; | |
| char buf[4096]; | |
| size_t offset = 0; | |
| while (offset < len) { | |
| size_t chunk = (len - offset) < (sizeof(buf) - 1) ? (len - offset) : (sizeof(buf) - 1); | |
| memcpy(buf, data + offset, chunk); | |
| buf[chunk] = '\0'; | |
| c->cb(buf, is_stderr, c->user_data); | |
| offset += chunk; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/c/example_common.h` around lines 80 - 87, Update the forward
function to process data in multiple chunks instead of truncating input beyond
the fixed buffer size. Loop until all len bytes are forwarded, copying each
segment into buf, null-terminating it, and invoking c->cb with the original
stream and user-data context.
Comment on lines
+46
to
+52
| - �?Structured error handling (error codes + messages) | ||
| - �?OCI container images | ||
| - �?Hardware-accelerated VMs (KVM/Hypervisor.framework) | ||
| - �?Command execution with streaming output | ||
| - �?Box lifecycle management | ||
| - �?Performance metrics | ||
| - �?Multi-box management |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='sdks/c/README.md'
echo '--- file type ---'
file "$file" || true
echo
echo '--- line numbers around reported ranges ---'
sed -n '40,60p;310,330p;650,675p;715,725p;790,805p;845,855p;870,880p;998,1015p' "$file" | cat -n
echo
echo '--- suspect characters (show unicode code points via Python) ---'
python3 - <<'PY'
from pathlib import Path
p = Path('sdks/c/README.md')
text = p.read_text(encoding='utf-8', errors='replace')
targets = ['�', '�?']
for i, line in enumerate(text.splitlines(), 1):
if '�' in line:
print(f'LINE {i}: {line}')
print(' codepoints:', ' '.join(f'U+{ord(ch):04X}' for ch in line))
PY
echo
echo '--- raw bytes around first occurrence ---'
python3 - <<'PY'
from pathlib import Path
data = Path('sdks/c/README.md').read_bytes()
needle = '�'.encode('utf-8', 'replace') # placeholder; not used
# search for utf-8 replacement char bytes EF BF BD
pat = b'\xef\xbf\xbd'
idx = data.find(pat)
print('first replacement-char-byte-index:', idx)
if idx != -1:
start = max(0, idx - 80)
end = min(len(data), idx + 160)
chunk = data[start:end]
print(chunk)
print('hex:', chunk.hex(' '))
PYRepository: boxlite-ai/boxlite
Length of output: 18589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='sdks/c/README.md'
echo '--- line numbers around reported ranges ---'
sed -n '40,60p;310,330p;650,675p;715,725p;790,805p;845,855p;870,880p;998,1015p' "$file" | cat -n
echo
echo '--- suspect characters (show unicode code points via Python) ---'
python3 - <<'PY'
from pathlib import Path
p = Path('sdks/c/README.md')
text = p.read_text(encoding='utf-8', errors='replace')
for i, line in enumerate(text.splitlines(), 1):
if '�' in line:
print(f'LINE {i}: {line}')
print(' codepoints:', ' '.join(f'U+{ord(ch):04X}' for ch in line))
PY
echo
echo '--- raw bytes around first occurrence ---'
python3 - <<'PY'
from pathlib import Path
data = Path('sdks/c/README.md').read_bytes()
pat = b'\xef\xbf\xbd'
idx = data.find(pat)
print('first replacement-char-byte-index:', idx)
if idx != -1:
start = max(0, idx - 80)
end = min(len(data), idx + 160)
chunk = data[start:end]
print(chunk)
print('hex:', chunk.hex(' '))
PYRepository: boxlite-ai/boxlite
Length of output: 18532
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='sdks/c/README.md'
echo '--- file type ---'
file "$file" || true
echo
echo '--- reported ranges ---'
for range in '40,60p' '310,330p' '650,675p' '715,725p' '790,805p' '845,855p' '870,880p' '998,1015p'; do
echo "### sed -n $range"
sed -n "$range" "$file" | cat -n
done
echo
echo '--- lines containing U+FFFD replacement char ---'
python3 - <<'PY'
from pathlib import Path
text = Path('sdks/c/README.md').read_text(encoding='utf-8', errors='replace')
for i, line in enumerate(text.splitlines(), 1):
if '\ufffd' in line:
print(f'{i}: {line}')
PY
echo
echo '--- byte scan for UTF-8 replacement char sequence EF BF BD ---'
python3 - <<'PY'
from pathlib import Path
data = Path('sdks/c/README.md').read_bytes()
pat = b'\xef\xbf\xbd'
count = data.count(pat)
print('count:', count)
if count:
idx = data.find(pat)
print('first_index:', idx)
start = max(0, idx - 64)
end = min(len(data), idx + 128)
chunk = data[start:end]
print(chunk.hex(' '))
PYRepository: boxlite-ai/boxlite
Length of output: 7175
Fix the corrupted README glyphs. Multiple sections still render as �? — the feature list, memory-management mappings, threading note, platform table, migration headings, and architecture diagram. Replace them with valid UTF-8 glyphs or plain ASCII equivalents.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdks/c/README.md` around lines 46 - 52, Replace all corrupted `�?` glyphs
throughout README.md—including the feature list, memory-management mappings,
threading note, platform table, migration headings, and architecture
diagram—with valid UTF-8 symbols or plain ASCII equivalents, preserving the
surrounding content and formatting.
Comment on lines
+20
to
+64
| // Free `err` only when a call failed; matches the README convention and keeps | ||
| // the test free of double-free concerns across successive assertions. | ||
| #define FREE_IF_ERR(code, err) do { if ((code) != Ok) boxlite_error_free(&(err)); } while (0) | ||
|
|
||
| int main(void) { | ||
| CBoxliteOptions *opts = NULL; | ||
| CBoxliteError err = {0}; | ||
|
|
||
| // Build an options object — no runtime, no VM, no I/O. | ||
| BoxliteErrorCode c = boxlite_options_new("alpine:3.19", &opts, &err); | ||
| CHECK(c == Ok, "boxlite_options_new succeeds"); | ||
| if (c != Ok) { boxlite_error_free(&err); return 1; } | ||
|
|
||
| // host_port = 0 means "same as guest_port" (per boxlite.h) — valid. | ||
| err = (CBoxliteError){0}; | ||
| c = boxlite_options_add_port(opts, /*host_port*/ 0, /*guest_port*/ 8080, | ||
| BoxlitePortProtocolTcp, /*host_ip*/ NULL); | ||
| CHECK(c == Ok, "add_port(host=0, guest=8080, Tcp, NULL) -> Ok"); | ||
| FREE_IF_ERR(c, err); | ||
|
|
||
| // Explicit host bind + UDP — valid. | ||
| err = (CBoxliteError){0}; | ||
| c = boxlite_options_add_port(opts, 9090, 90, BoxlitePortProtocolUdp, "127.0.0.1"); | ||
| CHECK(c == Ok, "add_port(9090, 90, Udp, 127.0.0.1) -> Ok"); | ||
| FREE_IF_ERR(c, err); | ||
|
|
||
| // guest_port == 0 is invalid (guest_port required, 1..65535). | ||
| err = (CBoxliteError){0}; | ||
| c = boxlite_options_add_port(opts, 0, 0, BoxlitePortProtocolTcp, NULL); | ||
| CHECK(c == InvalidArgument, "add_port guest_port=0 -> InvalidArgument"); | ||
| FREE_IF_ERR(c, err); | ||
|
|
||
| // NULL opts must be rejected, not crash. | ||
| err = (CBoxliteError){0}; | ||
| c = boxlite_options_add_port(NULL, 8080, 80, BoxlitePortProtocolTcp, NULL); | ||
| CHECK(c == InvalidArgument, "add_port NULL opts -> InvalidArgument"); | ||
| FREE_IF_ERR(c, err); | ||
|
|
||
| // Non-UTF-8 host_ip must be rejected (header contract). | ||
| const char bad_utf8[] = { (char)0xff, (char)0xfe, 0 }; | ||
| err = (CBoxliteError){0}; | ||
| c = boxlite_options_add_port(opts, 8080, 80, BoxlitePortProtocolTcp, bad_utf8); | ||
| CHECK(c == InvalidArgument, "add_port non-UTF-8 host_ip -> InvalidArgument"); | ||
| FREE_IF_ERR(c, err); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove unnecessary error-freeing logic and use meaningful variable names.
boxlite_options_add_port does not take a CBoxliteError output parameter. Resetting err to {0} and attempting to free it via FREE_IF_ERR is logically incorrect and misleading, as the error struct is never populated by these calls. As per coding guidelines, use meaningful variable and function names that clearly describe their purpose by renaming c to code (or status).
♻️ Proposed fixes
-// Free `err` only when a call failed; matches the README convention and keeps
-// the test free of double-free concerns across successive assertions.
-#define FREE_IF_ERR(code, err) do { if ((code) != Ok) boxlite_error_free(&(err)); } while (0)
-
int main(void) {
CBoxliteOptions *opts = NULL;
CBoxliteError err = {0};
// Build an options object — no runtime, no VM, no I/O.
- BoxliteErrorCode c = boxlite_options_new("alpine:3.19", &opts, &err);
- CHECK(c == Ok, "boxlite_options_new succeeds");
- if (c != Ok) { boxlite_error_free(&err); return 1; }
+ BoxliteErrorCode code = boxlite_options_new("alpine:3.19", &opts, &err);
+ CHECK(code == Ok, "boxlite_options_new succeeds");
+ if (code != Ok) { boxlite_error_free(&err); return 1; }
// host_port = 0 means "same as guest_port" (per boxlite.h) — valid.
- err = (CBoxliteError){0};
- c = boxlite_options_add_port(opts, /*host_port*/ 0, /*guest_port*/ 8080,
- BoxlitePortProtocolTcp, /*host_ip*/ NULL);
- CHECK(c == Ok, "add_port(host=0, guest=8080, Tcp, NULL) -> Ok");
- FREE_IF_ERR(c, err);
+ code = boxlite_options_add_port(opts, /*host_port*/ 0, /*guest_port*/ 8080,
+ BoxlitePortProtocolTcp, /*host_ip*/ NULL);
+ CHECK(code == Ok, "add_port(host=0, guest=8080, Tcp, NULL) -> Ok");
// Explicit host bind + UDP — valid.
- err = (CBoxliteError){0};
- c = boxlite_options_add_port(opts, 9090, 90, BoxlitePortProtocolUdp, "127.0.0.1");
- CHECK(c == Ok, "add_port(9090, 90, Udp, 127.0.0.1) -> Ok");
- FREE_IF_ERR(c, err);
+ code = boxlite_options_add_port(opts, 9090, 90, BoxlitePortProtocolUdp, "127.0.0.1");
+ CHECK(code == Ok, "add_port(9090, 90, Udp, 127.0.0.1) -> Ok");
// guest_port == 0 is invalid (guest_port required, 1..65535).
- err = (CBoxliteError){0};
- c = boxlite_options_add_port(opts, 0, 0, BoxlitePortProtocolTcp, NULL);
- CHECK(c == InvalidArgument, "add_port guest_port=0 -> InvalidArgument");
- FREE_IF_ERR(c, err);
+ code = boxlite_options_add_port(opts, 0, 0, BoxlitePortProtocolTcp, NULL);
+ CHECK(code == InvalidArgument, "add_port guest_port=0 -> InvalidArgument");
// NULL opts must be rejected, not crash.
- err = (CBoxliteError){0};
- c = boxlite_options_add_port(NULL, 8080, 80, BoxlitePortProtocolTcp, NULL);
- CHECK(c == InvalidArgument, "add_port NULL opts -> InvalidArgument");
- FREE_IF_ERR(c, err);
+ code = boxlite_options_add_port(NULL, 8080, 80, BoxlitePortProtocolTcp, NULL);
+ CHECK(code == InvalidArgument, "add_port NULL opts -> InvalidArgument");
// Non-UTF-8 host_ip must be rejected (header contract).
const char bad_utf8[] = { (char)0xff, (char)0xfe, 0 };
- err = (CBoxliteError){0};
- c = boxlite_options_add_port(opts, 8080, 80, BoxlitePortProtocolTcp, bad_utf8);
- CHECK(c == InvalidArgument, "add_port non-UTF-8 host_ip -> InvalidArgument");
- FREE_IF_ERR(c, err);
+ code = boxlite_options_add_port(opts, 8080, 80, BoxlitePortProtocolTcp, bad_utf8);
+ CHECK(code == InvalidArgument, "add_port non-UTF-8 host_ip -> InvalidArgument");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Free `err` only when a call failed; matches the README convention and keeps | |
| // the test free of double-free concerns across successive assertions. | |
| #define FREE_IF_ERR(code, err) do { if ((code) != Ok) boxlite_error_free(&(err)); } while (0) | |
| int main(void) { | |
| CBoxliteOptions *opts = NULL; | |
| CBoxliteError err = {0}; | |
| // Build an options object — no runtime, no VM, no I/O. | |
| BoxliteErrorCode c = boxlite_options_new("alpine:3.19", &opts, &err); | |
| CHECK(c == Ok, "boxlite_options_new succeeds"); | |
| if (c != Ok) { boxlite_error_free(&err); return 1; } | |
| // host_port = 0 means "same as guest_port" (per boxlite.h) — valid. | |
| err = (CBoxliteError){0}; | |
| c = boxlite_options_add_port(opts, /*host_port*/ 0, /*guest_port*/ 8080, | |
| BoxlitePortProtocolTcp, /*host_ip*/ NULL); | |
| CHECK(c == Ok, "add_port(host=0, guest=8080, Tcp, NULL) -> Ok"); | |
| FREE_IF_ERR(c, err); | |
| // Explicit host bind + UDP — valid. | |
| err = (CBoxliteError){0}; | |
| c = boxlite_options_add_port(opts, 9090, 90, BoxlitePortProtocolUdp, "127.0.0.1"); | |
| CHECK(c == Ok, "add_port(9090, 90, Udp, 127.0.0.1) -> Ok"); | |
| FREE_IF_ERR(c, err); | |
| // guest_port == 0 is invalid (guest_port required, 1..65535). | |
| err = (CBoxliteError){0}; | |
| c = boxlite_options_add_port(opts, 0, 0, BoxlitePortProtocolTcp, NULL); | |
| CHECK(c == InvalidArgument, "add_port guest_port=0 -> InvalidArgument"); | |
| FREE_IF_ERR(c, err); | |
| // NULL opts must be rejected, not crash. | |
| err = (CBoxliteError){0}; | |
| c = boxlite_options_add_port(NULL, 8080, 80, BoxlitePortProtocolTcp, NULL); | |
| CHECK(c == InvalidArgument, "add_port NULL opts -> InvalidArgument"); | |
| FREE_IF_ERR(c, err); | |
| // Non-UTF-8 host_ip must be rejected (header contract). | |
| const char bad_utf8[] = { (char)0xff, (char)0xfe, 0 }; | |
| err = (CBoxliteError){0}; | |
| c = boxlite_options_add_port(opts, 8080, 80, BoxlitePortProtocolTcp, bad_utf8); | |
| CHECK(c == InvalidArgument, "add_port non-UTF-8 host_ip -> InvalidArgument"); | |
| FREE_IF_ERR(c, err); | |
| int main(void) { | |
| CBoxliteOptions *opts = NULL; | |
| CBoxliteError err = {0}; | |
| // Build an options object — no runtime, no VM, no I/O. | |
| BoxliteErrorCode code = boxlite_options_new("alpine:3.19", &opts, &err); | |
| CHECK(code == Ok, "boxlite_options_new succeeds"); | |
| if (code != Ok) { boxlite_error_free(&err); return 1; } | |
| // host_port = 0 means "same as guest_port" (per boxlite.h) — valid. | |
| code = boxlite_options_add_port(opts, /*host_port*/ 0, /*guest_port*/ 8080, | |
| BoxlitePortProtocolTcp, /*host_ip*/ NULL); | |
| CHECK(code == Ok, "add_port(host=0, guest=8080, Tcp, NULL) -> Ok"); | |
| // Explicit host bind + UDP — valid. | |
| code = boxlite_options_add_port(opts, 9090, 90, BoxlitePortProtocolUdp, "127.0.0.1"); | |
| CHECK(code == Ok, "add_port(9090, 90, Udp, 127.0.0.1) -> Ok"); | |
| // guest_port == 0 is invalid (guest_port required, 1..65535). | |
| code = boxlite_options_add_port(opts, 0, 0, BoxlitePortProtocolTcp, NULL); | |
| CHECK(code == InvalidArgument, "add_port guest_port=0 -> InvalidArgument"); | |
| // NULL opts must be rejected, not crash. | |
| code = boxlite_options_add_port(NULL, 8080, 80, BoxlitePortProtocolTcp, NULL); | |
| CHECK(code == InvalidArgument, "add_port NULL opts -> InvalidArgument"); | |
| // Non-UTF-8 host_ip must be rejected (header contract). | |
| const char bad_utf8[] = { (char)0xff, (char)0xfe, 0 }; | |
| code = boxlite_options_add_port(opts, 8080, 80, BoxlitePortProtocolTcp, bad_utf8); | |
| CHECK(code == InvalidArgument, "add_port non-UTF-8 host_ip -> InvalidArgument"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdks/c/tests/test_options_port.c` around lines 20 - 64, Remove the err
resets, FREE_IF_ERR macro, and related calls from the boxlite_options_add_port
assertions because that API does not populate a CBoxliteError; retain error
cleanup only for boxlite_options_new where applicable. Rename the status
variable c to code throughout main, including its declaration, assignments,
checks, and messages, while preserving the existing result assertions.
Source: Coding guidelines
Member
|
Hi @xxxzl114514. Thanks for filing this PR. Please take a look at the comments |
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
2 participants
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.
Summary
Closes #1002.
examples/c/example_common.h— ported to the post-and-drain callback API.boxlite_create_boxnow receives the handle via aCBoxCreateBoxCb; exec usesboxlite_box_exec+on_stdout/on_stderr+execution_wait(exit code viaCExecutionWaitCb). The public helper signatures (create_alpine_box_or_exit,execute_and_wait) are unchanged, so existing examples that use them still call the same way. Added a newcreate_box_with_options_or_exit(runtime, opts)helper for examples that need custom options.examples/c/06_port_forwarding.c(new) — demonstrates the previously-uncoveredboxlite_options_add_port(host 127.0.0.1:8080 → guest 80) andboxlite_options_add_volume(read-only mount), using the fixed helper.sdks/c/tests/test_options_port.c(new) — VM-free FFI test (CTest labelffi) pinningboxlite_options_add_port's contract:Okfor valid args;InvalidArgumentforguest_port == 0,NULL opts, and non-UTF-8host_ip; plus a smoke call ofboxlite_options_add_volume.sdks/c/README.md— fix the clone URL (github.com/boxlite/boxlite→github.com/boxlite-ai/boxlite).Why
The C SDK migrated box creation / exec / lifecycle to a callback API, but
example_common.hwas not updated, so the examples stopped compiling (see #1002 for the gcc repro). Theadd_port/add_volumeoption API also had no coverage despite a recent breaking change. This PR fixes the shared helper and adds coverage.The test is deliberately VM-free (label
ffi, liketest_null_callback) so it runs in CI without a KVM runner.Test plan
gcc -fsyntax-only -Wall -Wextra -I examples/c -I sdks/c/include examples/c/06_port_forwarding.c→ cleanmake dev:ccd sdks/c/tests && mkdir -p build && cd build && cmake .. && makectest -L ffi -V→test_options_portpasses (no VM required)grep -rn "boxlite_options_add_port\|boxlite_options_add_volume" examples/c sdks/c/tests→ the new files are the only hits06_port_forwarding.cis intentionally not wired intoexamples/c/CMakeLists.txt: thatmakeis already broken onmainbecause the sibling examples (01_lifecycle, …) call the old direct lifecycle signatures (tracked separately). It is verified by thegcc -fsyntax-onlystep above and will be wired into the build once those siblings are migrated.Summary by CodeRabbit