#19554 closed defect (implemented)

Require libevent >= 2

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

Description

The supported Debian and Ubuntu versions all provide libevent 2 now, for OSX and WIndows it isn't an issue either, so we can drop support for libevent 1.4

Child Tickets

Change History (10)

comment:1 Changed 10 months ago by Sebastian

Branch libevent2 in my repository. It raises the required version to 2.0.10-stable or above. The rationale for not going with >2 .0.1 or even just > 2 is that this allows us to simplify a bunch more checks and all the distributions I've found all have at least 2.0.16. I didn't want to raise the dependency to 2.0.16 in case I missed anything, but 2.0.10 is the first stable release in the libevent release series.

I want to particularly highlight the "delete wrong comments" commit, because maybe the one comment I removed is not actually wrong. In that case maybe the behaviour of the function is strange and we should fix it.

comment:2 Changed 10 months ago by Sebastian

  • Status changed from new to needs_review

comment:3 Changed 10 months ago by cypherpunks

A review after some quick grepping and reading relevant header files.

src/common/compat_libevent.h

  • Remove the le_version_t typedef because it's unused now.
  • Update the following preprocessor function comment accordingly.

src/common/compat_libevent.c

  • Remove the comment at the top regarding older Libevent versions.
  • Remove the cast and comment on line 268.

src/common/procmon.c

  • Make poll_interval_tv const on line 166.
  • Remove the subsequent comment because it's obsolete now.

comment:4 Changed 10 months ago by cypherpunks

Some more suggestions.

src/common/compat_libevent.h

  • Include the Libevent headers event2/event.h and event2/bufferevent.h instead of declaring the Libevent structs.
  • Remove the event2/util.h include because it's already included by event2/event.h.
  • Remove the function aliases and replace instances in the rest of the code.

comment:5 Changed 10 months ago by Sebastian

Thanks you for the review, nice catches! I have followed all the suggestions except the function aliases one. I intentionally did not do that because I'm not sure that'd be appreciated due to touching many places in the code. If we want to do that I'll happily add it in

comment:6 Changed 10 months ago by Sebastian

And I added a changes file for good measure.

comment:7 Changed 10 months ago by cypherpunks

The file src/or/eventdns_tor.h can also be removed since nothing uses it anymore. Be sure to also remove it from the header list in src/or/include.am.

comment:8 Changed 10 months ago by Sebastian

addressed as well

comment:9 Changed 10 months ago by Sebastian

  • Milestone changed from Tor: 0.3.0.x-final to Tor: 0.2.9.x-final

Changing to 0.2.9.x as suggested by Nick

comment:10 Changed 10 months ago by nickm

  • Resolution set to implemented
  • Status changed from needs_review to closed

Merged!

Note: See TracTickets for help on using tickets.