From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 10 Jun 2025 16:29:40 +0000 (-0400)
Subject: Remove circuit_sendme_cell_is_next
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5af6c8e63eca5561c68534fb4dfba69e851c097f;p=thirdparty%2Ftor.git

Remove circuit_sendme_cell_is_next

We needed this function previously, when we only computed a SENDME
tag conditionally, depending on whether we were about to need it.
But as part of the CGO refactoring, we now compute and store SENDME
tags unconditionally, whenever a cell is originated or recognized.
Therefore this function is no longer needed anywhere.
---

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index be9c15ed39..4774a70610 100644
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -382,43 +382,6 @@ record_cell_digest_on_circ(circuit_t *circ,
  * Public API
  */
 
-/** Return true iff the next cell for the given cell window is expected to be
- * a SENDME.
- *
- * We are able to know that because the package or inflight window value minus
- * one cell (the possible SENDME cell) should be a multiple of the
- * cells-per-sendme increment value (set via consensus parameter, negotiated
- * for the circuit, and passed in as sendme_inc).
- *
- * This function is used when recording a cell digest and this is done quite
- * low in the stack when decrypting or encrypting a cell. The window is only
- * updated once the cell is actually put in the outbuf.
- */
-// XXXX Todo remove if truly not needed.
-ATTR_UNUSED STATIC bool
-circuit_sendme_cell_is_next(int deliver_window, int sendme_inc)
-{
-  /* Are we at the limit of the increment and if not, we don't expect next
-   * cell is a SENDME.
-   *
-   * We test against the window minus 1 because when we are looking if the
-   * next cell is a SENDME, the window (either package or deliver) hasn't been
-   * decremented just yet so when this is called, we are currently processing
-   * the "window - 1" cell.
-   *
-   * Because deliver_window starts at CIRCWINDOW_START and counts down,
-   * to get the actual number of received cells for this check, we must
-   * first convert to received cells, or the modulus operator will fail.
-   */
-  tor_assert(deliver_window <= CIRCWINDOW_START);
-  if (((CIRCWINDOW_START - (deliver_window - 1)) % sendme_inc) != 0) {
-    return false;
-  }
-
-  /* Next cell is expected to be a SENDME. */
-  return true;
-}
-
 /** Called when we've just received a relay data cell, when we've just
  * finished flushing all bytes to stream <b>conn</b>, or when we've flushed
  * *some* bytes to the stream <b>conn</b>.
diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h
index c76b6d3947..53c6776d76 100644
--- a/src/core/or/sendme.h
+++ b/src/core/or/sendme.h
@@ -72,8 +72,6 @@ STATIC bool sendme_is_valid(const circuit_t *circ,
                             const crypt_path_t *layer_hint,
                             const uint8_t *cell_payload,
                             size_t cell_payload_len);
-STATIC bool circuit_sendme_cell_is_next(int deliver_window,
-                                        int sendme_inc);
 
 #endif /* defined(TOR_UNIT_TESTS) */
 
diff --git a/src/test/test_sendme.c b/src/test/test_sendme.c
index cde6dd5d29..1eb3de5684 100644
--- a/src/test/test_sendme.c
+++ b/src/test/test_sendme.c
@@ -317,50 +317,6 @@ test_package_payload_len(void *arg)
   circuit_free(c);
 }
 
-/* Check that circuit_sendme_is_next works with a window of 1000,
- * and a sendme_inc of 100 (old school tor compat) */
-static void
-test_sendme_is_next1000(void *arg)
-{
- (void)arg;
- tt_int_op(circuit_sendme_cell_is_next(1000, 100), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(999, 100), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(901, 100), OP_EQ, 1);
-
- tt_int_op(circuit_sendme_cell_is_next(900, 100), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(899, 100), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(801, 100), OP_EQ, 1);
-
- tt_int_op(circuit_sendme_cell_is_next(101, 100), OP_EQ, 1);
- tt_int_op(circuit_sendme_cell_is_next(100, 100), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(99, 100), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(1, 100), OP_EQ, 1);
- tt_int_op(circuit_sendme_cell_is_next(0, 100), OP_EQ, 0);
-
-done:
- ;
-}
-
-/* Check that circuit_sendme_is_next works with a window of 31 */
-static void
-test_sendme_is_next(void *arg)
-{
- (void)arg;
- tt_int_op(circuit_sendme_cell_is_next(1000, 31), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(970, 31), OP_EQ, 1);
- tt_int_op(circuit_sendme_cell_is_next(969, 31), OP_EQ, 0);
-
- /* deliver_window should never get this low, but test anyway */
- tt_int_op(circuit_sendme_cell_is_next(9, 31), OP_EQ, 1);
- tt_int_op(circuit_sendme_cell_is_next(8, 31), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(7, 31), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(1, 31), OP_EQ, 0);
- tt_int_op(circuit_sendme_cell_is_next(0, 31), OP_EQ, 0);
-
- done:
-  ;
-}
-
 struct testcase_t sendme_tests[] = {
   { "v1_record_digest", test_v1_record_digest, TT_FORK,
     NULL, NULL },
@@ -371,8 +327,6 @@ struct testcase_t sendme_tests[] = {
   { "cell_version_validation", test_cell_version_validation, TT_FORK,
     NULL, NULL },
   { "package_payload_len", test_package_payload_len, 0, NULL, NULL },
-  { "sendme_is_next1000", test_sendme_is_next1000, 0, NULL, NULL },
-  { "sendme_is_next", test_sendme_is_next, 0, NULL, NULL },
 
   END_OF_TESTCASES
 };