#13827: Cell handling code duplication in channel.c -----------------------------------------------+--------------------------- Reporter: rl1987 | Owner: pingl Type: defect | Status: | needs_revision Priority: Medium | Milestone: Tor: | 0.3.0.x-final Component: Core Tor/Tor | Version: Severity: Normal | Resolution: Keywords: refactoring, easy, review-group-9 | Actual Points: Parent ID: | Points: 0.1 Reviewer: dgoulet | Sponsor: -----------------------------------------------+--------------------------- Changes (by nickm):
* status: needs_review => needs_revision * milestone: Tor: 0.2.9.x-final => Tor: 0.3.0.x-final Comment: More fundamentally, I think my issue with the channel.c patch is that it doesn't actually reduce the code duplication very much. There are still 3 CHANNEL_IS_CLOSING() checks that are more or less the same, 6 log statements that are more or less the same, etc. And there's a loss of type safety -- we should be scared every time we use void *. Here's what I'd suggest: Have an underlying function that all 3 functions call. It could look like this: {{{ static int channel_write_cell_generic_(channel_t *chan, const char *celltype, void *cellptr, cell_queue_entry_t *q) { tor_assert... if (CHANNEL_IS_CLOSING(chan)) { log... return; } log... channel_write_cell_queue_entry(chan, q); } void channel_write_var_cell(channel_t *chan, var_cell_t *var_cell) { cell_queue_entry_t q; q.type = CELL_QUEUE_VAR, q.u.var_cell = var_cell; channel_write_cell_generic(chan, "var_cell_t", var_cell, &q); } ... }}} -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13827#comment:15> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs