Opened 3 years ago

Closed 3 years ago

#21891 closed enhancement (fixed)

hs: Refactoring of some part of the legacy code for prop224 usage

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, prop224
Cc: Actual Points:
Parent ID: #21888 Points: 1
Reviewer: Sponsor: SponsorR-must

Description

This branch contains 4 commits that we need for prop224 service implementation work (#20657).

First commit is a trivial refactor to make a function in rendservice.c public and broader to both HS subsystems (legacy and new).

Second and third commit are code moving and minor cleanup.

Fourth commit is a bit more involving that is making the pruning rendservice list public so the HS prop224 code can use it later on with #20657 work.

Child Tickets

Change History (8)

comment:1 Changed 3 years ago by dgoulet

Status: newneeds_review

See branch: ticket21891_031_01
Gitlab review: https://gitlab.com/dgoulet/tor/merge_requests/20

comment:2 Changed 3 years ago by dgoulet

Parent ID: #21888

comment:3 Changed 3 years ago by asn

Initial review done here. Nothing serious found.

That pruning commit was kinda hard to go through as I was completely unfamiliar with the original mechanics.

comment:4 Changed 3 years ago by nickm

Status: needs_reviewneeds_revision

I had a couple of comments on the final commit, but otherwise looks good.

comment:5 Changed 3 years ago by dgoulet

I've addressed all the comments but some discussions are clearly not over so I'll push an updated branch once we arrive to a consensus.

comment:6 Changed 3 years ago by dgoulet

Status: needs_revisionneeds_review

Everything has been addressed. Two fixup commits out of it.

See branch: ticket21891_031_01

FYI, there is going to be a slight conflict when merging because of the trunnel renaming that just happened but very easy to fix.

comment:7 Changed 3 years ago by dgoulet

(Quickly, extra fixup commit on top fixing comments still mentioning "temporary list")

comment:8 Changed 3 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Squashing and merging!

Note: See TracTickets for help on using tickets.