refactor: remove dead code, fix silent socket option no-ops, fix tempdir leak - #37
Conversation
There was a problem hiding this comment.
Pull request overview
Cleanup PR that removes a duplicated FsSandbox::register method (~190 LOC) and updates its tests to exercise the production FsRouter code path, converts silent no-op behavior in net_setsockopt / net_getsockopt for unknown options into explicit errors, and fixes a TempDir leak in pptx-gen by returning the TempDir to the caller for proper lifetime ownership.
Changes:
- Delete dead
FsSandbox::registerand migrate two tests toFsRouterwith a/hostpreopen. - Return errors for unsupported
(level, optname)pairs innet_setsockopt/net_getsockopt. - Return
(PathBuf, TempDir)frominject_script_into_rootfsinstead ofmem::forget-ing theTempDir.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| host/src/lib.rs | Drops FsSandbox::register duplication, updates two FS tests to use FsRouter/Preopen, and replaces silent socket-option no-ops with explicit errors. |
| demos/pptx-gen/src/main.rs | Returns the TempDir from inject_script_into_rootfs so the caller controls its lifetime; updates caller to bind it. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Linux Benchmarks
Details
| Benchmark suite | Current: 6c578d8 | Previous: 2ad29b8 | Ratio |
|---|---|---|---|
hello_world (median) |
20 ms |
20 ms |
1 |
pandas (median) |
100 ms |
110 ms |
0.91 |
density (per VM) |
7 MB |
7 MB |
1 |
snapshot (disk) |
385 MiB |
385 MiB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Windows Benchmarks
Details
| Benchmark suite | Current: 6c578d8 | Previous: 2ad29b8 | Ratio |
|---|---|---|---|
hello_world (median) |
255 ms |
316 ms |
0.81 |
pandas (median) |
815 ms |
1015 ms |
0.80 |
density (per VM) |
6 MB |
6 MB |
1 |
snapshot (disk) |
393 MiB |
393 MiB |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: danbugs <danilochiarlone@gmail.com>
Signed-off-by: danbugs <danilochiarlone@gmail.com>
Signed-off-by: danbugs <danilochiarlone@gmail.com>
Signed-off-by: danbugs <danilochiarlone@gmail.com>
Signed-off-by: danbugs <danilochiarlone@gmail.com>
Summary
Three cleanup items:
FsSandbox::registerwhich duplicated all 9 fs handlers from
FsRouter::register. Only used intests — updated those to use
FsRouterwith a single preopen, testing theactual production code path.
net_setsockoptandnet_getsockoptsilentlyreturned Ok/0 for unrecognized (level, optname) pairs. Now returns an error so
guests can detect unsupported options.
inject_script_into_rootfsusedstd::mem::forget(temp_dir)to keep the returned path valid. Now returns theTempDiralongside the path so the caller owns the lifetime.Test plan
cargo checkpasses for demos/pptx-gen