[tor-commits] [tor/master] hs-v2: Modernize rend_client_circuit_cleanup() code

[asn at torproject.org]

commit 1aafe3376ea08fb31fcb7e0a8a390ff5edeb8602
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed Oct 30 15:17:47 2019 -0400

    hs-v2: Modernize rend_client_circuit_cleanup() code
    
    Old and messy code path. Structure it in a more pleasant and readable way. No
    behavior change with this refactor.
    
    Part of #32020
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/feature/rend/rendclient.c | 85 +++++++++++++++++++++++++------------------
 src/feature/rend/rendclient.h |  2 +-
 2 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/src/feature/rend/rendclient.c b/src/feature/rend/rendclient.c
index 971839825..632b00c85 100644
--- a/src/feature/rend/rendclient.c
+++ b/src/feature/rend/rendclient.c
@@ -1254,47 +1254,62 @@ rend_parse_service_authorization(const or_options_t *options,
 /** The given circuit is being freed. Take appropriate action if it is of
  * interest to the client subsystem. */
 void
-rend_client_circuit_cleanup(circuit_t *circ)
+rend_client_circuit_cleanup(const circuit_t *circ)
 {
-  int reason = circ->marked_for_close_reason;
-  int orig_reason = circ->marked_for_close_orig_reason;
+  int reason, orig_reason;
+  bool has_timed_out, ip_is_redundant;
+  const origin_circuit_t *ocirc = NULL;
 
   tor_assert(circ);
+  tor_assert(CIRCUIT_IS_ORIGIN(circ));
 
-  if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT) {
-    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
-    int timed_out = (reason == END_CIRC_REASON_TIMEOUT);
+  reason = circ->marked_for_close_reason;
+  orig_reason = circ->marked_for_close_orig_reason;
+  ocirc = CONST_TO_ORIGIN_CIRCUIT(circ);
+  tor_assert(ocirc->rend_data);
+
+  has_timed_out = (reason == END_CIRC_REASON_TIMEOUT);
+  ip_is_redundant = (orig_reason == END_CIRC_REASON_IP_NOW_REDUNDANT);
+
+  switch (circ->purpose) {
+  case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT:
+  {
+    if (ip_is_redundant) {
+      break;
+    }
     tor_assert(circ->state == CIRCUIT_STATE_OPEN);
     tor_assert(ocirc->build_state->chosen_exit);
-    if (orig_reason != END_CIRC_REASON_IP_NOW_REDUNDANT &&
-        ocirc->rend_data) {
-      /* treat this like getting a nack from it */
-      log_info(LD_REND, "Failed intro circ %s to %s (awaiting ack). %s",
-          safe_str_client(rend_data_get_address(ocirc->rend_data)),
-          safe_str_client(build_state_get_exit_nickname(ocirc->build_state)),
-          timed_out ? "Recording timeout." : "Removing from descriptor.");
-      rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
-                                             ocirc->rend_data,
-                                             timed_out ?
-                                             INTRO_POINT_FAILURE_TIMEOUT :
-                                             INTRO_POINT_FAILURE_GENERIC);
-    }
-  } else if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCING &&
-             reason != END_CIRC_REASON_TIMEOUT) {
-    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
-    if (ocirc->build_state->chosen_exit && ocirc->rend_data) {
-      if (orig_reason != END_CIRC_REASON_IP_NOW_REDUNDANT &&
-          ocirc->rend_data) {
-        log_info(LD_REND, "Failed intro circ %s to %s "
-            "(building circuit to intro point). "
-            "Marking intro point as possibly unreachable.",
-            safe_str_client(rend_data_get_address(ocirc->rend_data)),
-            safe_str_client(build_state_get_exit_nickname(
-                                              ocirc->build_state)));
-        rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
-                                              ocirc->rend_data,
-                                              INTRO_POINT_FAILURE_UNREACHABLE);
-      }
+    /* Treat this like getting a nack from it */
+    log_info(LD_REND, "Failed intro circ %s to %s (awaiting ack). %s",
+        safe_str_client(rend_data_get_address(ocirc->rend_data)),
+        safe_str_client(build_state_get_exit_nickname(ocirc->build_state)),
+        has_timed_out ? "Recording timeout." : "Removing from descriptor.");
+    rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
+                                           ocirc->rend_data,
+                                           has_timed_out ?
+                                           INTRO_POINT_FAILURE_TIMEOUT :
+                                           INTRO_POINT_FAILURE_GENERIC);
+    break;
+  }
+  case CIRCUIT_PURPOSE_C_INTRODUCING:
+  {
+    /* Ignore if we were introducing and it timed out, we didn't pick an exit
+     * point yet (IP) or the reason indicate that it was a redundant IP. */
+    if (has_timed_out || !ocirc->build_state->chosen_exit || ip_is_redundant) {
+      break;
     }
+    log_info(LD_REND, "Failed intro circ %s to %s "
+             "(building circuit to intro point). "
+             "Marking intro point as possibly unreachable.",
+             safe_str_client(rend_data_get_address(ocirc->rend_data)),
+             safe_str_client(build_state_get_exit_nickname(
+                                                  ocirc->build_state)));
+    rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
+                                           ocirc->rend_data,
+                                           INTRO_POINT_FAILURE_UNREACHABLE);
+    break;
+  }
+  default:
+    break;
   }
 }
diff --git a/src/feature/rend/rendclient.h b/src/feature/rend/rendclient.h
index dd9fa61ce..da6f4646d 100644
--- a/src/feature/rend/rendclient.h
+++ b/src/feature/rend/rendclient.h
@@ -47,7 +47,7 @@ rend_service_authorization_t *rend_client_lookup_service_authorization(
                                                 const char *onion_address);
 void rend_service_authorization_free_all(void);
 
-void rend_client_circuit_cleanup(circuit_t *circ);
+void rend_client_circuit_cleanup(const circuit_t *circ);
 
 #endif /* !defined(TOR_RENDCLIENT_H) */