Re: [tor-bugs] #23750 [Core Tor/Tor]: Isolate libevent usage to a few locations

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#23750: Isolate libevent usage to a few locations
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  nickm
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  refactoring, technical-debt,         |  Actual Points:
  review-group-31                                |
Parent ID:                                       |         Points:
 Reviewer:  dgoulet                              |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision
 * reviewer:   => dgoulet


Comment:

 From commit `6c5a4bef4e77cf37`:

 * I would propose that `mainloop_event_new()` validates at least the `cb`
 argument passed with a `tor_assert()` (seems it can't be NULL nor it makes
 sense to be NULL).

  I would also document the function that the `userdata` ownership is
 passed to the returned `mainloop_event_`.

 * Can we use `mainloop_event_activate()` if the event was never
 scheduled/added to the main loop?

 * Can I use `tv = NULL` in `mainloop_event_schedule()` to tell it "now" or
 `tv` must be a valid pointer?

 * I would document what this does if called multiple time:
 `mainloop_event_cancel()`.

 * Maybe we should have this using the `FREE_AND_NULL` macro scheme?
 `mainloop_event_free()`

 From commit `3edf64a95f91d9be`:

 * This function `threadpool_register_reply_event()` doesn't use the nice
 `mainloop_event_t` API and I can see that is because we create an event
 with specific flags (EV_PERSIST | EV_READ).

  And then it goes on using `event_add()` directly. Couldn't we remove all
 this and make it that it would use that mainloop interface created prior?

 * Also, why is `threadpool_register_reply_event()` is taking a base event,
 can't we abstract that to always use `tor_libevent_get_base()` or that
 doesn't play nice in multi thread?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23750#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs