fix: restore correct ScanResultRkyv with manual IpAddr serialization#22
Conversation
Co-authored-by: doublegate <6858123+doublegate@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
doublegate
left a comment
There was a problem hiding this comment.
Reviewed / Approved - changes to support PR #18 closeout -- DG 1/24
There was a problem hiding this comment.
Pull request overview
This pull request attempts to fix compilation failures caused by duplicate ScanResultRkyv definitions left from an incomplete merge of PR #21's rkyv migration. The PR removes 128 lines of code including one complete struct definition and its conversion implementations, consolidates duplicate deserialization logic in the mmap reader, and fixes a broken documentation link.
Changes:
- Removed duplicate
ScanResultRkyvstruct definition (122 lines) with manual byte serialization for IP addresses - Fixed duplicate length-prefix reading logic and removed references to non-existent
entry_bytesvariable in mmap_reader.rs - Changed a broken documentation URL to plain text reference
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/prtip-core/src/types.rs | Removed duplicate ScanResultRkyv definition with manual IP serialization; reformatted PortState enum derives |
| crates/prtip-scanner/src/output/mmap_reader.rs | Consolidated duplicate length-prefix reading, fixed entry_bytes reference issue |
| docs/archive/19-PHASE4-ENHANCEMENTS.md | Fixed broken findsec.org URL reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: doublegate <6858123+doublegate@users.noreply.github.com>
doublegate
left a comment
There was a problem hiding this comment.
Reviewed / Approved - corrected version with manual byte serialization (target_ip_bytes: [u8; 16] + is_ipv4: bool) -- DG 1/25
doublegate
left a comment
There was a problem hiding this comment.
Approved (No Changes) - PR was reading from older variant of code-base, ready to proceed -- DG 1/25
5d48440
into
dependabot/cargo/cargo-920260e05e
PR #21's rkyv migration was incompletely merged, leaving two conflicting
ScanResultRkyvdefinitions that broke compilation and CodeQL analysis. Initial fix removed the wrong duplicate - this PR corrects that mistake.Changes
Restored correct struct definition (with manual serialization,
crates/prtip-core/src/types.rs)target_ip_bytes: [u8; 16]andis_ipv4: boolIpAddrdirectlystd::net::IpAddrdoes not implement rkyv'sArchive,Serialize, andDeserializetraitsFixed broken deserialization code (
crates/prtip-scanner/src/output/mmap_reader.rs)entry_bytesvariableFixed dead link (
docs/archive/19-PHASE4-ENHANCEMENTS.md)Context
The correct implementation requires manual serialization because
std::net::IpAddrlacks rkyv trait implementations:The manual serialization converts
IpAddrto/from byte arrays andPortStateto/from u8 in theFromtrait implementations, enabling rkyv's zero-copy deserialization in the memory-mapped file writer/reader.Net: +80 lines (correct implementation), resolves compilation errors and prevents runtime serialization failures.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.