Opened 9 months ago

Closed 9 months ago

#25162 closed defect (implemented)

Perhaps, use zstd's "static-only" functions

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

Description

I think maybe we can make this safe if we explicitly check that the compile version matches the runtime version. (Thanks to Hello71 for the tip)

Child Tickets

Change History (6)

comment:1 Changed 9 months ago by nickm

Status: assignedneeds_review

See my branch 25162_zstd_static. What do you think?

comment:2 Changed 9 months ago by ahf

Status: needs_reviewmerge_ready
  • 358b609e9dfd503cc25985d197a92610cbbe9f3d looks good, but maybe it would make sense to have a configure option to make it easy to test if the code still works when ZSTD_STATIC_LINKING_ONLY is disabled if that is a concern?
  • 7cb954209d2c3b12d0d60a46a74f2c31ad6d48c5 looks good.
  • f98cb5d3552666ede73137d998162094d5a31a1a looks good.
  • a77a366b87fb04a878392284241fe72850b4dd88 looks good.

comment:3 Changed 9 months ago by nickm

Component: - Select a componentCore Tor/Tor

Okay, I've added the configure option. If you still like it, let's take this in 0.3.4.

comment:4 Changed 9 months ago by ahf

Looks good to me.

comment:5 Changed 9 months ago by Hello71

so maybe I'm missing something, but I'm pretty sure tor_zstd_get_version_str should just call ZSTD_versionString? or is that just because it doesn't work on debian stable? if so, we should comment that.

comment:6 Changed 9 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged to master, which is now 0.3.4.

(I think it's okay to use ZSTD_versionString(), but I don't think the current approach hurts anything.)

Note: See TracTickets for help on using tickets.