Opened 11 days ago

Closed 4 days ago

#25185 closed enhancement (implemented)

Create utilities for using Rust static strings in C

Reported by: isis Owned by: isis
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: rust, review-group-32
Cc: komlo Actual Points:
Parent ID: Points: 1
Reviewer: catalyst Sponsor: SponsorM

Description

This is a continuation of #25127, to provide some utilities for working with static strings across the FFI boundary without accidentally introducing leaks.

Child Tickets

Change History (5)

comment:1 Changed 9 days ago by isis

Status: assignedneeds_review

The eventual thing I came up with is a lot cleaner than the fix for #25127, with the same amount of code now (but drastically less code for each future time we run into this issue). It's in my bug25185 branch and Travis is passing.

comment:2 Changed 7 days ago by nickm

Keywords: review-group-32 added

comment:3 Changed 7 days ago by catalyst

Reviewer: catalyst

comment:4 Changed 6 days ago by catalyst

Status: needs_reviewmerge_ready

I think this looks good, though I might be missing subtle things because I'm still rather new to Rust.

Minor documentation clarity nits:

  • The "Note" in the documentation for cstr! refers to "the above code compiles", but it actually means the first out of three examples rather than the example immediately preceding it. Maybe replace it with "the first example above compiles"?
  • In that same "Note", "symbols table" should probably be "symbol table" (unless it's idiomatic in Rust documentation to use plural "symbols" there)

These are fairly minor so feel free to merge as is or without further review after fixing the nits.

comment:5 Changed 4 days ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged to master (which is now 0.3.4) with documentation tweaks as suggested.

Note: See TracTickets for help on using tickets.