Opened 3 years ago

Closed 3 years ago

#21919 closed defect (fixed)

hs: Change trunnel prop224 cell's namespace

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs, refactor, prop224
Cc: Actual Points:
Parent ID: #21888 Points: 0.2
Reviewer: Sponsor: SponsorR-must

Description (last modified by dgoulet)

Currently, the trunnel namespace for hidden service cells (in src/trunnel/hs/) is prefixed with hs_cell_*. We want to change this for two reasons.

First, if we could have something in the name indicating that it is trunnel, it would make it better for code semantic and separation.

Second, we want to create hs_cells.[ch] so we can put in there the cell creation/parsing/handling instead of growing hs_circuit.c to the "hydra size".

So for the renaming, here are some suggestions:

  1. tr_cell_*
  2. tr_hs_cell_*
  3. trunnel_cell_*
  4. trnl_cell_*

Considering that an ESTABLISH_INTRO or INTRODUCE1 cell is only for hidden service, probably the hs in there is superfluous?

Child Tickets

Change History (7)

comment:1 Changed 3 years ago by dgoulet

Description: modified (diff)

comment:2 Changed 3 years ago by dgoulet

From IRC:

< nickm> dgoulet: tr_ or trn_

comment:3 Changed 3 years ago by dgoulet

Another IRC vote:

< asn> dgoulet: i like trn_* and trnl_* but no strong opinion

comment:4 Changed 3 years ago by dgoulet

Status: newneeds_review

trn_ is the winner.

See branch: ticket21919_031_01.

@nickm, I'm a bit confused about this, does it need a change file for this super internal change?

Last edited 3 years ago by dgoulet (previous) (diff)

comment:5 Changed 3 years ago by asn

Patch LGTM.

There is only one instance of check-spaces that should be addressed:
Wide:./src/or/hs_intropoint.c:97

comment:6 Changed 3 years ago by dgoulet

Status: needs_reviewmerge_ready

Pushed an extra commit to address that. Thanks! Moving to merge_ready.

See branch: ticket21919_031_01.

comment:7 Changed 3 years ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merging!

Note: See TracTickets for help on using tickets.