[tor-bugs] #13827 [Core Tor/Tor]: Cell handling code duplication in channel.c

[Tor Bug Tracker & Wiki]

#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