Merge branch 'ticket7356_squashed'

changes/bug7356

@@ -0,0 +1,5 @@
+  o Code simplifications and refactoring:
+    - Add inline functions and convenience macros for quick lookup of
+      state component of channel_t structure. Refactor various parts of
+      codebase to use convenience macros instead of checking state
+      member of channel_t directly. Fixes issue 7356.

src/or/channel.c

@@ -417,8 +417,7 @@ channel_register(channel_t *chan)
   smartlist_add(all_channels, chan);
 
   /* Is it finished? */
-  if (chan->state == CHANNEL_STATE_CLOSED ||
-      chan->state == CHANNEL_STATE_ERROR) {
+  if (CHANNEL_FINISHED(chan)) {
     /* Put it in the finished list, creating it if necessary */
     if (!finished_channels) finished_channels = smartlist_new();
     smartlist_add(finished_channels, chan);
@@ -427,7 +426,7 @@ channel_register(channel_t *chan)
     if (!active_channels) active_channels = smartlist_new();
     smartlist_add(active_channels, chan);
 
-    if (chan->state != CHANNEL_STATE_CLOSING) {
+    if (!CHANNEL_IS_CLOSING(chan)) {
       /* It should have a digest set */
       if (!tor_digest_is_zero(chan->identity_digest)) {
         /* Yeah, we're good, add it to the map */
@@ -462,8 +461,7 @@ channel_unregister(channel_t *chan)
   if (!(chan->registered)) return;
 
   /* Is it finished? */
-  if (chan->state == CHANNEL_STATE_CLOSED ||
-      chan->state == CHANNEL_STATE_ERROR) {
+  if (CHANNEL_FINISHED(chan)) {
     /* Get it out of the finished list */
     if (finished_channels) smartlist_remove(finished_channels, chan);
   } else {
@@ -479,9 +477,7 @@ channel_unregister(channel_t *chan)
 
   /* Should it be in the digest map? */
   if (!tor_digest_is_zero(chan->identity_digest) &&
-      !(chan->state == CHANNEL_STATE_CLOSING ||
-        chan->state == CHANNEL_STATE_CLOSED ||
-        chan->state == CHANNEL_STATE_ERROR)) {
+      !(CHANNEL_CONDEMNED(chan))) {
     /* Remove it */
     channel_remove_from_digest_map(chan);
   }
@@ -581,9 +577,7 @@ channel_add_to_digest_map(channel_t *chan)
   tor_assert(chan);
 
   /* Assert that the state makes sense */
-  tor_assert(!(chan->state == CHANNEL_STATE_CLOSING ||
-               chan->state == CHANNEL_STATE_CLOSED ||
-               chan->state == CHANNEL_STATE_ERROR));
+  tor_assert(!CHANNEL_CONDEMNED(chan));
 
   /* Assert that there is a digest */
   tor_assert(!tor_digest_is_zero(chan->identity_digest));
@@ -821,8 +815,8 @@ channel_free(channel_t *chan)
   if (!chan) return;
 
   /* It must be closed or errored */
-  tor_assert(chan->state == CHANNEL_STATE_CLOSED ||
-             chan->state == CHANNEL_STATE_ERROR);
+  tor_assert(CHANNEL_FINISHED(chan));
+
   /* It must be deregistered */
   tor_assert(!(chan->registered));
 
@@ -1036,9 +1030,7 @@ channel_get_cell_handler(channel_t *chan)
 {
   tor_assert(chan);
 
-  if (chan->state == CHANNEL_STATE_OPENING ||
-      chan->state == CHANNEL_STATE_OPEN ||
-      chan->state == CHANNEL_STATE_MAINT)
+  if (CHANNEL_CAN_HANDLE_CELLS(chan))
     return chan->cell_handler;
 
   return NULL;
@@ -1056,9 +1048,7 @@ channel_get_var_cell_handler(channel_t *chan)
 {
   tor_assert(chan);
 
-  if (chan->state == CHANNEL_STATE_OPENING ||
-      chan->state == CHANNEL_STATE_OPEN ||
-      chan->state == CHANNEL_STATE_MAINT)
+  if (CHANNEL_CAN_HANDLE_CELLS(chan))
     return chan->var_cell_handler;
 
   return NULL;
@@ -1081,9 +1071,7 @@ channel_set_cell_handlers(channel_t *chan,
   int try_again = 0;
 
   tor_assert(chan);
-  tor_assert(chan->state == CHANNEL_STATE_OPENING ||
-             chan->state == CHANNEL_STATE_OPEN ||
-             chan->state == CHANNEL_STATE_MAINT);
+  tor_assert(CHANNEL_CAN_HANDLE_CELLS(chan));
 
   log_debug(LD_CHANNEL,
            "Setting cell_handler callback for channel %p to %p",
@@ -1137,9 +1125,8 @@ channel_mark_for_close(channel_t *chan)
   tor_assert(chan->close != NULL);
 
   /* If it's already in CLOSING, CLOSED or ERROR, this is a no-op */
-  if (chan->state == CHANNEL_STATE_CLOSING ||
-      chan->state == CHANNEL_STATE_CLOSED ||
-      chan->state == CHANNEL_STATE_ERROR) return;
+  if (CHANNEL_CONDEMNED(chan))
+    return;
 
   log_debug(LD_CHANNEL,
             "Closing channel %p (global ID " U64_FORMAT ") "
@@ -1218,9 +1205,8 @@ channel_close_from_lower_layer(channel_t *chan)
   tor_assert(chan != NULL);
 
   /* If it's already in CLOSING, CLOSED or ERROR, this is a no-op */
-  if (chan->state == CHANNEL_STATE_CLOSING ||
-      chan->state == CHANNEL_STATE_CLOSED ||
-      chan->state == CHANNEL_STATE_ERROR) return;
+  if (CHANNEL_CONDEMNED(chan))
+    return;
 
   log_debug(LD_CHANNEL,
             "Closing channel %p (global ID " U64_FORMAT ") "
@@ -1278,9 +1264,8 @@ channel_close_for_error(channel_t *chan)
   tor_assert(chan != NULL);
 
   /* If it's already in CLOSING, CLOSED or ERROR, this is a no-op */
-  if (chan->state == CHANNEL_STATE_CLOSING ||
-      chan->state == CHANNEL_STATE_CLOSED ||
-      chan->state == CHANNEL_STATE_ERROR) return;
+  if (CHANNEL_CONDEMNED(chan))
+    return;
 
   log_debug(LD_CHANNEL,
             "Closing channel %p due to lower-layer error",
@@ -1336,13 +1321,11 @@ void
 channel_closed(channel_t *chan)
 {
   tor_assert(chan);
-  tor_assert(chan->state == CHANNEL_STATE_CLOSING ||
-             chan->state == CHANNEL_STATE_CLOSED ||
-             chan->state == CHANNEL_STATE_ERROR);
+  tor_assert(CHANNEL_CONDEMNED(chan));
 
   /* No-op if already inactive */
-  if (chan->state == CHANNEL_STATE_CLOSED ||
-      chan->state == CHANNEL_STATE_ERROR) return;
+  if (CHANNEL_FINISHED(chan))
+    return;
 
   /* Inform any pending (not attached) circs that they should
    * give up. */
@@ -1405,10 +1388,7 @@ channel_clear_identity_digest(channel_t *chan)
             "global ID " U64_FORMAT,
             chan, U64_PRINTF_ARG(chan->global_identifier));
 
-  state_not_in_map =
-    (chan->state == CHANNEL_STATE_CLOSING ||
-     chan->state == CHANNEL_STATE_CLOSED ||
-     chan->state == CHANNEL_STATE_ERROR);
+  state_not_in_map = CHANNEL_CONDEMNED(chan);
 
   if (!state_not_in_map && chan->registered &&
       !tor_digest_is_zero(chan->identity_digest))
@@ -1441,10 +1421,8 @@ channel_set_identity_digest(channel_t *chan,
             identity_digest ?
               hex_str(identity_digest, DIGEST_LEN) : "(null)");
 
-  state_not_in_map =
-    (chan->state == CHANNEL_STATE_CLOSING ||
-     chan->state == CHANNEL_STATE_CLOSED ||
-     chan->state == CHANNEL_STATE_ERROR);
+  state_not_in_map = CHANNEL_CONDEMNED(chan);
+
   was_in_digest_map =
     !state_not_in_map &&
     chan->registered &&
@@ -1494,10 +1472,7 @@ channel_clear_remote_end(channel_t *chan)
             "global ID " U64_FORMAT,
             chan, U64_PRINTF_ARG(chan->global_identifier));
 
-  state_not_in_map =
-    (chan->state == CHANNEL_STATE_CLOSING ||
-     chan->state == CHANNEL_STATE_CLOSED ||
-     chan->state == CHANNEL_STATE_ERROR);
+  state_not_in_map = CHANNEL_CONDEMNED(chan);
 
   if (!state_not_in_map && chan->registered &&
       !tor_digest_is_zero(chan->identity_digest))
@@ -1533,10 +1508,8 @@ channel_set_remote_end(channel_t *chan,
             identity_digest ?
               hex_str(identity_digest, DIGEST_LEN) : "(null)");
 
-  state_not_in_map =
-    (chan->state == CHANNEL_STATE_CLOSING ||
-     chan->state == CHANNEL_STATE_CLOSED ||
-     chan->state == CHANNEL_STATE_ERROR);
+  state_not_in_map = CHANNEL_CONDEMNED(chan);
+
   was_in_digest_map =
     !state_not_in_map &&
     chan->registered &&
@@ -1761,9 +1734,7 @@ channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q)
   tor_assert(q);
 
   /* Assert that the state makes sense for a cell write */
-  tor_assert(chan->state == CHANNEL_STATE_OPENING ||
-             chan->state == CHANNEL_STATE_OPEN ||
-             chan->state == CHANNEL_STATE_MAINT);
+  tor_assert(CHANNEL_CAN_HANDLE_CELLS(chan));
 
   {
     circid_t circ_id;
@@ -1777,7 +1748,7 @@ channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q)
 
   /* Can we send it right out?  If so, try */
   if (TOR_SIMPLEQ_EMPTY(&chan->outgoing_queue) &&
-      chan->state == CHANNEL_STATE_OPEN) {
+      CHANNEL_IS_OPEN(chan)) {
     /* Pick the right write function for this cell type and save the result */
     switch (q->type) {
       case CELL_QUEUE_FIXED:
@@ -1835,7 +1806,7 @@ channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q)
     /* Update channel queue size */
     chan->bytes_in_queue += cell_bytes;
     /* Try to process the queue? */
-    if (chan->state == CHANNEL_STATE_OPEN) channel_flush_cells(chan);
+    if (CHANNEL_IS_OPEN(chan)) channel_flush_cells(chan);
   }
 }
 
@@ -1856,7 +1827,7 @@ channel_write_cell(channel_t *chan, cell_t *cell)
   tor_assert(chan);
   tor_assert(cell);
 
-  if (chan->state == CHANNEL_STATE_CLOSING) {
+  if (CHANNEL_IS_CLOSING(chan)) {
     log_debug(LD_CHANNEL, "Discarding cell_t %p on closing channel %p with "
               "global ID "U64_FORMAT, cell, chan,
               U64_PRINTF_ARG(chan->global_identifier));
@@ -1893,7 +1864,7 @@ channel_write_packed_cell(channel_t *chan, packed_cell_t *packed_cell)
   tor_assert(chan);
   tor_assert(packed_cell);
 
-  if (chan->state == CHANNEL_STATE_CLOSING) {
+  if (CHANNEL_IS_CLOSING(chan)) {
     log_debug(LD_CHANNEL, "Discarding packed_cell_t %p on closing channel %p "
               "with global ID "U64_FORMAT, packed_cell, chan,
               U64_PRINTF_ARG(chan->global_identifier));
@@ -1932,7 +1903,7 @@ channel_write_var_cell(channel_t *chan, var_cell_t *var_cell)
   tor_assert(chan);
   tor_assert(var_cell);
 
-  if (chan->state == CHANNEL_STATE_CLOSING) {
+  if (CHANNEL_IS_CLOSING(chan)) {
     log_debug(LD_CHANNEL, "Discarding var_cell_t %p on closing channel %p "
               "with global ID "U64_FORMAT, var_cell, chan,
               U64_PRINTF_ARG(chan->global_identifier));
@@ -2211,7 +2182,7 @@ channel_flush_some_cells, (channel_t *chan, ssize_t num_cells))
   if (!unlimited && num_cells <= flushed) goto done;
 
   /* If we aren't in CHANNEL_STATE_OPEN, nothing goes through */
-  if (chan->state == CHANNEL_STATE_OPEN) {
+  if (CHANNEL_IS_OPEN(chan)) {
     /* Try to flush as much as we can that's already queued */
     flushed += channel_flush_some_cells_from_outgoing_queue(chan,
         (unlimited ? -1 : num_cells - flushed));
@@ -2302,7 +2273,7 @@ channel_flush_some_cells_from_outgoing_queue(channel_t *chan,
   if (!unlimited && num_cells <= flushed) return 0;
 
   /* If we aren't in CHANNEL_STATE_OPEN, nothing goes through */
-  if (chan->state == CHANNEL_STATE_OPEN) {
+  if (CHANNEL_IS_OPEN(chan)) {
     while ((unlimited || num_cells > flushed) &&
            NULL != (q = TOR_SIMPLEQ_FIRST(&chan->outgoing_queue))) {
       free_q = 0;
@@ -2667,9 +2638,8 @@ channel_process_cells(channel_t *chan)
 {
   cell_queue_entry_t *q;
   tor_assert(chan);
-  tor_assert(chan->state == CHANNEL_STATE_CLOSING ||
-             chan->state == CHANNEL_STATE_MAINT ||
-             chan->state == CHANNEL_STATE_OPEN);
+  tor_assert(CHANNEL_IS_CLOSING(chan) || CHANNEL_IS_MAINT(chan) ||
+             CHANNEL_IS_OPEN(chan));
 
   log_debug(LD_CHANNEL,
             "Processing as many incoming cells as we can for channel %p",
@@ -2736,7 +2706,7 @@ channel_queue_cell(channel_t *chan, cell_t *cell)
 
   tor_assert(chan);
   tor_assert(cell);
-  tor_assert(chan->state == CHANNEL_STATE_OPEN);
+  tor_assert(CHANNEL_IS_OPEN(chan));
 
   /* Do we need to queue it, or can we just call the handler right away? */
   if (!(chan->cell_handler)) need_to_queue = 1;
@@ -2790,7 +2760,7 @@ channel_queue_var_cell(channel_t *chan, var_cell_t *var_cell)
 
   tor_assert(chan);
   tor_assert(var_cell);
-  tor_assert(chan->state == CHANNEL_STATE_OPEN);
+  tor_assert(CHANNEL_IS_OPEN(chan));
 
   /* Do we need to queue it, or can we just call the handler right away? */
   if (!(chan->var_cell_handler)) need_to_queue = 1;
@@ -2913,10 +2883,7 @@ channel_send_destroy(circid_t circ_id, channel_t *chan, int reason)
   }
 
   /* Check to make sure we can send on this channel first */
-  if (!(chan->state == CHANNEL_STATE_CLOSING ||
-        chan->state == CHANNEL_STATE_CLOSED ||
-        chan->state == CHANNEL_STATE_ERROR) &&
-      chan->cmux) {
+  if (!CHANNEL_CONDEMNED(chan) && chan->cmux) {
     channel_note_destroy_pending(chan, circ_id);
     circuitmux_append_destroy_cell(chan, chan->cmux, circ_id, reason);
     log_debug(LD_OR,
@@ -3106,9 +3073,7 @@ channel_free_list(smartlist_t *channels, int mark_for_close)
     }
     channel_unregister(curr);
     if (mark_for_close) {
-      if (!(curr->state == CHANNEL_STATE_CLOSING ||
-            curr->state == CHANNEL_STATE_CLOSED ||
-            curr->state == CHANNEL_STATE_ERROR)) {
+      if (!CHANNEL_CONDEMNED(curr)) {
         channel_mark_for_close(curr);
       }
       channel_force_free(curr);
@@ -3322,9 +3287,7 @@ channel_get_for_extend(const char *digest,
     tor_assert(tor_memeq(chan->identity_digest,
                          digest, DIGEST_LEN));
 
-    if (chan->state == CHANNEL_STATE_CLOSING ||
-        chan->state == CHANNEL_STATE_CLOSED ||
-        chan->state == CHANNEL_STATE_ERROR)
+   if (CHANNEL_CONDEMNED(chan))
       continue;
 
     /* Never return a channel on which the other end appears to be
@@ -3334,7 +3297,7 @@ channel_get_for_extend(const char *digest,
     }
 
     /* Never return a non-open connection. */
-    if (chan->state != CHANNEL_STATE_OPEN) {
+    if (!CHANNEL_IS_OPEN(chan)) {
       /* If the address matches, don't launch a new connection for this
        * circuit. */
       if (channel_matches_target_addr_for_extend(chan, target_addr))

src/or/channel.h

@@ -502,6 +502,39 @@ channel_t * channel_find_by_remote_digest(const char *identity_digest);
  */
 channel_t * channel_next_with_digest(channel_t *chan);
 
+/*
+ * Helper macros to lookup state of given channel.
+ */
+
+#define CHANNEL_IS_CLOSED(chan) (channel_is_in_state((chan), \
+                                 CHANNEL_STATE_CLOSED))
+#define CHANNEL_IS_OPENING(chan) (channel_is_in_state((chan), \
+                                  CHANNEL_STATE_OPENING))
+#define CHANNEL_IS_OPEN(chan) (channel_is_in_state((chan), \
+                               CHANNEL_STATE_OPEN))
+#define CHANNEL_IS_MAINT(chan) (channel_is_in_state((chan), \
+                                CHANNEL_STATE_MAINT))
+#define CHANNEL_IS_CLOSING(chan) (channel_is_in_state((chan), \
+                                  CHANNEL_STATE_CLOSING))
+#define CHANNEL_IS_ERROR(chan) (channel_is_in_state((chan), \
+                                CHANNEL_STATE_ERROR))
+
+#define CHANNEL_FINISHED(chan) (CHANNEL_IS_CLOSED(chan) || \
+                                CHANNEL_IS_ERROR(chan))
+
+#define CHANNEL_CONDEMNED(chan) (CHANNEL_IS_CLOSING(chan) || \
+                                 CHANNEL_FINISHED(chan))
+
+#define CHANNEL_CAN_HANDLE_CELLS(chan) (CHANNEL_IS_OPENING(chan) || \
+                                        CHANNEL_IS_OPEN(chan) || \
+                                        CHANNEL_IS_MAINT(chan))
+
+static INLINE int
+channel_is_in_state(channel_t *chan, channel_state_t state)
+{
+  return chan->state == state;
+}
+
 /*
  * Metadata queries/updates
  */

src/or/channeltls.c

@@ -940,13 +940,13 @@ channel_tls_handle_state_change_on_orconn(channel_tls_t *chan,
 
   base_chan = TLS_CHAN_TO_BASE(chan);
 
-  /* Make sure the base connection state makes sense - shouldn't be error,
-   * closed or listening. */
+  /* Make sure the base connection state makes sense - shouldn't be error
+   * or closed. */
 
-  tor_assert(base_chan->state == CHANNEL_STATE_OPENING ||
-             base_chan->state == CHANNEL_STATE_OPEN ||
-             base_chan->state == CHANNEL_STATE_MAINT ||
-             base_chan->state == CHANNEL_STATE_CLOSING);
+  tor_assert(CHANNEL_IS_OPENING(base_chan) ||
+             CHANNEL_IS_OPEN(base_chan) ||
+             CHANNEL_IS_MAINT(base_chan) ||
+             CHANNEL_IS_CLOSING(base_chan));
 
   /* Did we just go to state open? */
   if (state == OR_CONN_STATE_OPEN) {
@@ -964,7 +964,7 @@ channel_tls_handle_state_change_on_orconn(channel_tls_t *chan,
      * Not open, so from CHANNEL_STATE_OPEN we go to CHANNEL_STATE_MAINT,
      * otherwise no change.
      */
-    if (base_chan->state == CHANNEL_STATE_OPEN) {
+    if (CHANNEL_IS_OPEN(base_chan)) {
       channel_change_state(base_chan, CHANNEL_STATE_MAINT);
     }
   }

src/or/circpathbias.c

@@ -768,8 +768,8 @@ pathbias_send_usable_probe(circuit_t *circ)
 
   /* Can't probe if the channel isn't open */
   if (circ->n_chan == NULL ||
-      (circ->n_chan->state != CHANNEL_STATE_OPEN
-       && circ->n_chan->state != CHANNEL_STATE_MAINT)) {
+      (!CHANNEL_IS_OPEN(circ->n_chan)
+       && !CHANNEL_IS_MAINT(circ->n_chan))) {
     log_info(LD_CIRC,
              "Skipping pathbias probe for circuit %d: Channel is not open.",
              ocirc->global_identifier);

src/or/circuitbuild.c

@@ -672,7 +672,7 @@ circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell,
   if (CIRCUIT_IS_ORIGIN(circ)) {
     /* Update began timestamp for circuits starting their first hop */
     if (TO_ORIGIN_CIRCUIT(circ)->cpath->state == CPATH_STATE_CLOSED) {
-      if (circ->n_chan->state != CHANNEL_STATE_OPEN) {
+      if (!CHANNEL_IS_OPEN(circ->n_chan)) {
         log_warn(LD_CIRC,
                  "Got first hop for a circuit without an opened channel. "
                  "State: %s.", channel_state_to_string(circ->n_chan->state));

src/or/circuitlist.c

@@ -1752,9 +1752,7 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
   if (circ->n_chan) {
     circuit_clear_cell_queue(circ, circ->n_chan);
     /* Only send destroy if the channel isn't closing anyway */
-    if (!(circ->n_chan->state == CHANNEL_STATE_CLOSING ||
-          circ->n_chan->state == CHANNEL_STATE_CLOSED ||
-          circ->n_chan->state == CHANNEL_STATE_ERROR)) {
+    if (!CHANNEL_CONDEMNED(circ->n_chan)) {
       channel_send_destroy(circ->n_circ_id, circ->n_chan, reason);
     }
     circuitmux_detach_circuit(circ->n_chan->cmux, circ);
@@ -1786,9 +1784,7 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
     if (or_circ->p_chan) {
       circuit_clear_cell_queue(circ, or_circ->p_chan);
       /* Only send destroy if the channel isn't closing anyway */
-      if (!(or_circ->p_chan->state == CHANNEL_STATE_CLOSING ||
-            or_circ->p_chan->state == CHANNEL_STATE_CLOSED ||
-            or_circ->p_chan->state == CHANNEL_STATE_ERROR)) {
+      if (!CHANNEL_CONDEMNED(or_circ->p_chan)) {
         channel_send_destroy(or_circ->p_circ_id, or_circ->p_chan, reason);
       }
       circuitmux_detach_circuit(or_circ->p_chan->cmux, circ);

src/or/connection.c

@@ -544,8 +544,7 @@ connection_free_(connection_t *conn)
                or_conn, TLS_CHAN_TO_BASE(or_conn->chan),
                U64_PRINTF_ARG(
                  TLS_CHAN_TO_BASE(or_conn->chan)->global_identifier));
-      if (!(TLS_CHAN_TO_BASE(or_conn->chan)->state == CHANNEL_STATE_CLOSED ||
-            TLS_CHAN_TO_BASE(or_conn->chan)->state == CHANNEL_STATE_ERROR)) {
+      if (!CHANNEL_FINISHED(TLS_CHAN_TO_BASE(or_conn->chan))) {
         channel_close_for_error(TLS_CHAN_TO_BASE(or_conn->chan));
       }
 

src/or/connection_or.c

@@ -1149,9 +1149,7 @@ connection_or_notify_error(or_connection_t *conn,
   if (conn->chan) {
     chan = TLS_CHAN_TO_BASE(conn->chan);
     /* Don't transition if we're already in closing, closed or error */
-    if (!(chan->state == CHANNEL_STATE_CLOSING ||
-          chan->state == CHANNEL_STATE_CLOSED ||
-          chan->state == CHANNEL_STATE_ERROR)) {
+    if (!CHANNEL_CONDEMNED(chan)) {
       channel_close_for_error(chan);
     }
   }
@@ -1310,9 +1308,7 @@ connection_or_close_normally(or_connection_t *orconn, int flush)
   if (orconn->chan) {
     chan = TLS_CHAN_TO_BASE(orconn->chan);
     /* Don't transition if we're already in closing, closed or error */
-    if (!(chan->state == CHANNEL_STATE_CLOSING ||
-          chan->state == CHANNEL_STATE_CLOSED ||
-          chan->state == CHANNEL_STATE_ERROR)) {
+    if (!CHANNEL_CONDEMNED(chan)) {
       channel_close_from_lower_layer(chan);
     }
   }
@@ -1333,9 +1329,7 @@ connection_or_close_for_error(or_connection_t *orconn, int flush)
   if (orconn->chan) {
     chan = TLS_CHAN_TO_BASE(orconn->chan);
     /* Don't transition if we're already in closing, closed or error */
-    if (!(chan->state == CHANNEL_STATE_CLOSING ||
-          chan->state == CHANNEL_STATE_CLOSED ||
-          chan->state == CHANNEL_STATE_ERROR)) {
+    if (!CHANNEL_CONDEMNED(chan)) {
       channel_close_for_error(chan);
     }
   }