Opened 5 years ago

Closed 5 years ago

#13399 closed defect (fixed)

New digest256map_t type; use it for microdescriptors.

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We store microdescriptors by a 256-bit SHA256 digest of their text. But in some places (e.g. microdescs_add_to_cache) we use a 160-bit prefix of the SHA256 digest. We should just add a digst256map_t type, and use it as appropriate.

Child Tickets

Change History (6)

comment:1 Changed 5 years ago by nickm

Status: newneeds_review

I have an implementation of digest256map_t in a branch "digest256map".

This branch actually reduces the amount of code in container.c by using macros to instantiate strmap and digestmap, so that we can avoid cut-and-pasting the same code a third time into digest256map.

Note that previously, our unit tests had complete line coverage for all the strmap/digestmap functions except for digestmap_assert_ok(). So the new code ought to have high coverage too, even though it's macro-generated.

This does not implement the whole ticket here, but I'd like to get it reviewed and merged soon, since I think #12498 wants it.

comment:2 Changed 5 years ago by yawning

Looks good to me on cursory inspection.

comment:3 Changed 5 years ago by nickm

Status: needs_reviewnew

Great; merged that! Re-newing this ticket for solution of the microdescriptor issue.

comment:4 Changed 5 years ago by nickm

Status: newneeds_review

See branch "bug13399" in my public repository for a couple of microdescriptor fixes here.

comment:5 Changed 5 years ago by sysrqb

Looks like a relatively simple replacement, and it builds. lgtm

comment:6 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

thanks; merged it! (I've also tried it and made sure that it seems to work fwict.)

Note: See TracTickets for help on using tickets.