Conversation
vibix_libc exposes C-ABI symbols (open, openat, read, write, close, stat, fstat, lstat, mkdir, rmdir, unlink, link, symlink, readlink, chmod, chown, rename, getcwd, chdir) that delegate to vibix_abi syscall wrappers. This allows the upstream libc and nix crates to link against vibix. vibix_libc_defs provides type definitions (struct stat, timespec, sigaction, dirent64, utsname, iovec) and constants (O_RDONLY, O_CREAT, S_IFMT, S_IFDIR, errno values, syscall numbers, etc.) matching the Linux x86_64 layout that vibix uses. Both crates are added to the workspace Cargo.toml and pass cargo check. The kernel still builds cleanly via cargo xtask build. Closes #855 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces ChangesLibc Infrastructure: Shim and Type Definitions
Sequence DiagramsequenceDiagram
participant UC as Userspace Code
participant VL as vibix_libc<br/>(C-ABI Wrapper)
participant VA as vibix_abi<br/>(syscall! macro)
participant K as Kernel
UC->>VL: call open(path, flags, mode)<br/>#[no_mangle] extern "C"
VL->>VL: syscall_ret converter ready
VL->>VA: syscall!(SYS_OPEN, ...)
VA->>K: trigger syscall instruction
K-->>VA: return i64 result
VA-->>VL: raw syscall result
VL->>VL: ret < 0?<br/>set ERRNO = -ret,<br/>return -1
VL-->>UC: return i32 (error or fd)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
userspace/vibix_libc/src/fcntl.rs (1)
6-8: ⚡ Quick winConsider importing syscall numbers from
vibix_libc_defs.
SYS_OPENandSYS_OPENATare already defined invibix_libc_defs/src/lib.rs(lines 249 and 287). Duplicating these constants creates a maintenance burden. Addingvibix_libc_defsas a dependency would allow importing these constants and ensure consistency.♻️ Suggested refactor
In
userspace/vibix_libc/Cargo.toml, add the dependency:[dependencies] vibix_abi = { path = "../vibix_abi" } +vibix_libc_defs = { path = "../vibix_libc_defs" }Then in
fcntl.rs:use crate::helpers::syscall_ret; use vibix_abi::syscall; +use vibix_libc_defs::{SYS_OPEN, SYS_OPENAT}; -// Syscall numbers (Linux x86_64) -const SYS_OPEN: u64 = 2; -const SYS_OPENAT: u64 = 257;🤖 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 `@userspace/vibix_libc/src/fcntl.rs` around lines 6 - 8, Replace the duplicated syscall constants SYS_OPEN and SYS_OPENAT in fcntl.rs with imports from the shared crate: add vibix_libc_defs as a dependency in userspace/vibix_libc/Cargo.toml, then remove the local const definitions and import the constants (e.g., use vibix_libc_defs::SYS_OPEN; use vibix_libc_defs::SYS_OPENAT;) so the file references the canonical definitions SYS_OPEN and SYS_OPENAT from vibix_libc_defs.userspace/vibix_libc/src/stat.rs (1)
7-11: ⚡ Quick winConsolidate syscall-number constants to a single source of truth.
These values are already exported by
vibix_libc_defs; redefining them here increases drift risk between crates. Importing shared constants keeps ABI numbers synchronized.Proposed refactor
use crate::helpers::syscall_ret; use vibix_abi::syscall; +use vibix_libc_defs::{SYS_CHMOD, SYS_CHOWN, SYS_FSTAT, SYS_LSTAT, SYS_STAT}; -// Syscall numbers (Linux x86_64) -const SYS_STAT: u64 = 4; -const SYS_FSTAT: u64 = 5; -const SYS_LSTAT: u64 = 6; -const SYS_CHMOD: u64 = 90; -const SYS_CHOWN: u64 = 92;🤖 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 `@userspace/vibix_libc/src/stat.rs` around lines 7 - 11, The syscall-number constants SYS_STAT, SYS_FSTAT, SYS_LSTAT, SYS_CHMOD, and SYS_CHOWN are duplicated here; replace these local definitions by importing the canonical constants from vibix_libc_defs and update any references in this module to use those imported symbols (e.g., add a use vibix_libc_defs::{SYS_STAT, SYS_FSTAT, SYS_LSTAT, SYS_CHMOD, SYS_CHOWN}; or reference them with the full path). Remove the local const declarations so the module relies on the single source of truth and keeps ABI numbers synchronized.userspace/vibix_libc/src/unistd.rs (1)
8-19: ⚡ Quick winReuse syscall constants from
vibix_libc_defsinstead of redefining them.Keeping numbers duplicated in this module can drift from the definitions crate and create hard-to-debug ABI mismatches. Prefer importing the shared constants.
Proposed refactor
use crate::helpers::syscall_ret; use vibix_abi::syscall; +use vibix_libc_defs::{ + SYS_CHDIR, SYS_CLOSE, SYS_GETCWD, SYS_LINK, SYS_MKDIR, SYS_READ, SYS_READLINK, + SYS_RENAME, SYS_RMDIR, SYS_SYMLINK, SYS_UNLINK, SYS_WRITE, +}; -// Syscall numbers (Linux x86_64) -const SYS_READ: u64 = 0; -const SYS_WRITE: u64 = 1; -const SYS_CLOSE: u64 = 3; -const SYS_GETCWD: u64 = 79; -const SYS_CHDIR: u64 = 80; -const SYS_RENAME: u64 = 82; -const SYS_MKDIR: u64 = 83; -const SYS_RMDIR: u64 = 84; -const SYS_LINK: u64 = 86; -const SYS_UNLINK: u64 = 87; -const SYS_SYMLINK: u64 = 88; -const SYS_READLINK: u64 = 89;🤖 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 `@userspace/vibix_libc/src/unistd.rs` around lines 8 - 19, Replace the duplicated numeric syscall constants with imports from the shared definitions crate: remove the local const declarations (SYS_READ, SYS_WRITE, SYS_CLOSE, SYS_GETCWD, SYS_CHDIR, SYS_RENAME, SYS_MKDIR, SYS_RMDIR, SYS_LINK, SYS_UNLINK, SYS_SYMLINK, SYS_READLINK) and instead import them from vibix_libc_defs (e.g., add a use vibix_libc_defs::{SYS_READ, SYS_WRITE, ...} or a glob import) so this module reuses the canonical syscall constants and avoids drift.
🤖 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 `@userspace/vibix_libc/src/lib.rs`:
- Around line 21-22: The doc comment is inaccurate — it claims to re-export
"defs" crate types but the code re-exports the vibix_abi crate; either update
the comment to state that vibix_abi is being re-exported or actually re-export
vibix_libc_defs by adding it as a dependency and changing the pub use
accordingly; locate the pub use vibix_abi statement and replace or adjust its
comment or the symbol to match (use vibix_abi or pub use vibix_libc_defs) so the
comment and the exported crate name (vibix_abi / vibix_libc_defs) are
consistent.
---
Nitpick comments:
In `@userspace/vibix_libc/src/fcntl.rs`:
- Around line 6-8: Replace the duplicated syscall constants SYS_OPEN and
SYS_OPENAT in fcntl.rs with imports from the shared crate: add vibix_libc_defs
as a dependency in userspace/vibix_libc/Cargo.toml, then remove the local const
definitions and import the constants (e.g., use vibix_libc_defs::SYS_OPEN; use
vibix_libc_defs::SYS_OPENAT;) so the file references the canonical definitions
SYS_OPEN and SYS_OPENAT from vibix_libc_defs.
In `@userspace/vibix_libc/src/stat.rs`:
- Around line 7-11: The syscall-number constants SYS_STAT, SYS_FSTAT, SYS_LSTAT,
SYS_CHMOD, and SYS_CHOWN are duplicated here; replace these local definitions by
importing the canonical constants from vibix_libc_defs and update any references
in this module to use those imported symbols (e.g., add a use
vibix_libc_defs::{SYS_STAT, SYS_FSTAT, SYS_LSTAT, SYS_CHMOD, SYS_CHOWN}; or
reference them with the full path). Remove the local const declarations so the
module relies on the single source of truth and keeps ABI numbers synchronized.
In `@userspace/vibix_libc/src/unistd.rs`:
- Around line 8-19: Replace the duplicated numeric syscall constants with
imports from the shared definitions crate: remove the local const declarations
(SYS_READ, SYS_WRITE, SYS_CLOSE, SYS_GETCWD, SYS_CHDIR, SYS_RENAME, SYS_MKDIR,
SYS_RMDIR, SYS_LINK, SYS_UNLINK, SYS_SYMLINK, SYS_READLINK) and instead import
them from vibix_libc_defs (e.g., add a use vibix_libc_defs::{SYS_READ,
SYS_WRITE, ...} or a glob import) so this module reuses the canonical syscall
constants and avoids drift.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba8fe3fd-77b8-4be7-b05f-1383cb6f99f8
📒 Files selected for processing (10)
Cargo.tomluserspace/vibix_libc/Cargo.tomluserspace/vibix_libc/src/errno.rsuserspace/vibix_libc/src/fcntl.rsuserspace/vibix_libc/src/helpers.rsuserspace/vibix_libc/src/lib.rsuserspace/vibix_libc/src/stat.rsuserspace/vibix_libc/src/unistd.rsuserspace/vibix_libc_defs/Cargo.tomluserspace/vibix_libc_defs/src/lib.rs
| /// Re-export the defs crate types for convenience. | ||
| pub use vibix_abi; |
There was a problem hiding this comment.
Fix misleading documentation comment.
The comment says "Re-export the defs crate types" but the code re-exports vibix_abi, not vibix_libc_defs. Either update the comment to accurately describe what's being re-exported, or add vibix_libc_defs as a dependency and re-export it instead.
✏️ Suggested fix
-/// Re-export the defs crate types for convenience.
+/// Re-export the ABI crate for convenience.
pub use vibix_abi;📝 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.
| /// Re-export the defs crate types for convenience. | |
| pub use vibix_abi; | |
| /// Re-export the ABI crate for convenience. | |
| pub use vibix_abi; |
🤖 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 `@userspace/vibix_libc/src/lib.rs` around lines 21 - 22, The doc comment is
inaccurate — it claims to re-export "defs" crate types but the code re-exports
the vibix_abi crate; either update the comment to state that vibix_abi is being
re-exported or actually re-export vibix_libc_defs by adding it as a dependency
and changing the pub use accordingly; locate the pub use vibix_abi statement and
replace or adjust its comment or the symbol to match (use vibix_abi or pub use
vibix_libc_defs) so the comment and the exported crate name (vibix_abi /
vibix_libc_defs) are consistent.
Summary
userspace/vibix_libc/crate that exposes C-ABI symbols (open,openat,read,write,close,stat,fstat,lstat,mkdir,rmdir,unlink,link,symlink,readlink,chmod,chown,rename,getcwd,chdir) delegating tovibix_abisyscall wrappersuserspace/vibix_libc_defs/crate providing Linux x86_64 ABI-compatible type definitions (struct stat,timespec,sigaction,dirent64,utsname,iovec) and constants (O_RDONLY,O_CREAT,S_IFMT,S_IFDIR, errno values, syscall numbers, clock IDs, etc.)Cargo.tomlThis is Phase 2/3 infrastructure per RFC 0009 -- enables the upstream
libcandnixcrates to link against vibix. Depends only onvibix_abi(PR #862, merged).Closes #855
Test plan
cargo check -p vibix_libc -p vibix_libc_defs-- clean (no errors, no warnings)cargo xtask build-- kernel still builds cleanly🤖 Generated with Claude Code