Opened 6 months ago

Closed 5 months ago

#24032 closed defect (implemented)

Add comments everywhere that C and Rust must stay in sync

Reported by: nickm Owned by: chelseakomlo
Priority: High Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: rust, doc
Cc: chelseakomlo Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

There are places in protover.rs where, if we were to change our C code without a corresponding change, we could break things. We should mark the C and the Rust code in all of those places.

Child Tickets

Change History (4)

comment:1 Changed 6 months ago by chelseakomlo

Owner: set to chelseakomlo
Status: newassigned

comment:2 Changed 6 months ago by chelseakomlo

See git@github.com:chelseakomlo/tor_patches.git, branch 24032-cargo-rust-sync. I can make a PR in oniongit if that makes review easier; these are all documentation changes.

I annotated only where data needs to stay in sync (such as enums and constants). If we change actual logic/behavior in src/or/protover, src/rust/protover will definitely need to change as well. I can put a comment in src/rust/protover/lib.rs to say that the overall module needs to track the C implementation if that would be helpful.

comment:3 Changed 6 months ago by chelseakomlo

Status: assignedneeds_review

comment:4 Changed 5 months ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged! I've also added corresponding comments in the C code, and a suggestion to do it in both places in the future. (Rationale: if the dependency is undocumented at either location, then it's likelier that somebody will edit that location without noticing the dependency.)

Note: See TracTickets for help on using tickets.