commit 2b10e99eb093e1e5e699db355e0c54342fa693b5 Author: Nick Mathewson nickm@torproject.org Date: Fri Oct 12 20:16:43 2012 -0400

Try refactoring channel list to use HT_ and LIST_ stuff directly --- src/or/channel.c | 191 +++++++++++++++++++++--------------------------------- src/or/channel.h | 4 +- 2 files changed, 75 insertions(+), 120 deletions(-)

diff --git a/src/or/channel.c b/src/or/channel.c index b594d05..7f3c6f5 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -81,7 +81,37 @@ static uint64_t n_channels_allocated = 0; * If more than one channel exists, follow the next_with_same_id pointer * as a linked list. */ -static digestmap_t *channel_identity_map = NULL; +HT_HEAD(channel_idmap, channel_idmap_entry_s) channel_identity_map = + HT_INITIALIZER(); + +typedef struct channel_idmap_entry_s { + HT_ENTRY(channel_idmap_entry_s) node; + uint8_t digest[DIGEST_LEN]; + LIST_HEAD(channel_list_s, channel_s) channel_list; +} channel_idmap_entry_t; + +static INLINE unsigned +channel_idmap_hash(const channel_idmap_entry_t *ent) +{ + const unsigned *a = (const unsigned *)ent->digest; +#if SIZEOF_INT == 4 + return a[0] ^ a[1] ^ a[2] ^ a[3] ^ a[4]; +#elif SIZEOF_INT == 8 + return a[0] ^ a[1]; +#endif +} + +static INLINE int +channel_idmap_eq(const channel_idmap_entry_t *a, + const channel_idmap_entry_t *b) +{ + return tor_memeq(a->digest, b->digest, DIGEST_LEN); +} + +HT_PROTOTYPE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash, + channel_idmap_eq); +HT_GENERATE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash, + channel_idmap_eq, 0.5, tor_malloc, tor_realloc, _tor_free);

static cell_queue_entry_t * cell_queue_entry_dup(cell_queue_entry_t *q); static void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off); @@ -505,7 +535,7 @@ channel_listener_unregister(channel_listener_t *chan_l) static void channel_add_to_digest_map(channel_t *chan) { - channel_t *tmp; + channel_idmap_entry_t *ent, search;

tor_assert(chan);

@@ -517,23 +547,15 @@ channel_add_to_digest_map(channel_t *chan) /* Assert that there is a digest */ tor_assert(!tor_digest_is_zero(chan->identity_digest));

- /* Allocate the identity map if we have to */ - if (!channel_identity_map) channel_identity_map = digestmap_new(); - - /* Insert it */ - tmp = digestmap_set(channel_identity_map, - chan->identity_digest, - chan); - if (tmp) { - /* There already was one, this goes at the head of the list */ - chan->next_with_same_id = tmp; - chan->prev_with_same_id = NULL; - tmp->prev_with_same_id = chan; - } else { - /* First with this digest */ - chan->next_with_same_id = NULL; - chan->prev_with_same_id = NULL; + memcpy(search.digest, chan->identity_digest, DIGEST_LEN); + ent = HT_FIND(channel_idmap, &channel_identity_map, &search); + if (! ent) { + ent = tor_malloc(sizeof(channel_idmap_entry_t)); + memcpy(ent->digest, chan->identity_digest, DIGEST_LEN); + LIST_INIT(&ent->channel_list); + HT_INSERT(channel_idmap, &channel_identity_map, ent); } + LIST_INSERT_HEAD(&ent->channel_list, chan, next_with_same_id);

log_debug(LD_CHANNEL, "Added channel %p (global ID " U64_FORMAT ") " @@ -553,13 +575,14 @@ channel_add_to_digest_map(channel_t *chan) static void channel_remove_from_digest_map(channel_t *chan) { - channel_t *tmp; + channel_idmap_entry_t *ent, search;

tor_assert(chan);

/* Assert that there is a digest */ tor_assert(!tor_digest_is_zero(chan->identity_digest));

+#if 0 /* Make sure we have a map */ if (!channel_identity_map) { /* @@ -584,66 +607,29 @@ channel_remove_from_digest_map(channel_t *chan)

return; } +#endif

- /* Look for it in the map */ - tmp = digestmap_get(channel_identity_map, chan->identity_digest); - if (tmp) { - /* Okay, it's here */ - /* Look for this channel */ - while (tmp && tmp != chan) { - tmp = tmp->next_with_same_id; - } + /* Pull it out of its list, wherever that list is */ + LIST_REMOVE(chan, next_with_same_id);

- if (tmp == chan) { - /* Found it, good */ - if (chan->next_with_same_id) { - chan->next_with_same_id->prev_with_same_id = chan->prev_with_same_id; - } - /* else we're the tail of the list */ - if (chan->prev_with_same_id) { - /* We're not the head of the list, so we can *just* unlink */ - chan->prev_with_same_id->next_with_same_id = chan->next_with_same_id; - } else { - /* We're the head, so we have to point the digest map entry at our - * next if we have one, or remove it if we're also the tail */ - if (chan->next_with_same_id) { - digestmap_set(channel_identity_map, - chan->identity_digest, - chan->next_with_same_id); - } else { - digestmap_remove(channel_identity_map, - chan->identity_digest); - } - } + memcpy(search.digest, chan->identity_digest, DIGEST_LEN); + ent = HT_FIND(channel_idmap, &channel_identity_map, &search);

- /* NULL out its next/prev pointers, and we're finished */ - chan->next_with_same_id = NULL; - chan->prev_with_same_id = NULL; + /* Look for it in the map */ + if (ent) { + /* Okay, it's here */

- log_debug(LD_CHANNEL, - "Removed channel %p (global ID " U64_FORMAT ") from " - "identity map in state %s (%d) with digest %s", - chan, U64_PRINTF_ARG(chan->global_identifier), - channel_state_to_string(chan->state), chan->state, - hex_str(chan->identity_digest, DIGEST_LEN)); - } else { - /* This is not good */ - log_warn(LD_BUG, - "Trying to remove channel %p (global ID " U64_FORMAT ") " - "with digest %s from identity map, but couldn't find it in " - "the list for that digest", - chan, U64_PRINTF_ARG(chan->global_identifier), - hex_str(chan->identity_digest, DIGEST_LEN)); - /* Unlink it and hope for the best */ - if (chan->next_with_same_id) { - chan->next_with_same_id->prev_with_same_id = chan->prev_with_same_id; - } - if (chan->prev_with_same_id) { - chan->prev_with_same_id->next_with_same_id = chan->next_with_same_id; - } - chan->next_with_same_id = NULL; - chan->prev_with_same_id = NULL; + if (LIST_EMPTY(&ent->channel_list)) { + HT_REMOVE(channel_idmap, &channel_identity_map, ent); + tor_free(ent); } + + log_debug(LD_CHANNEL, + "Removed channel %p (global ID " U64_FORMAT ") from " + "identity map in state %s (%d) with digest %s", + chan, U64_PRINTF_ARG(chan->global_identifier), + channel_state_to_string(chan->state), chan->state, + hex_str(chan->identity_digest, DIGEST_LEN)); } else { /* Shouldn't happen */ log_warn(LD_BUG, @@ -652,15 +638,6 @@ channel_remove_from_digest_map(channel_t *chan) "that digest", chan, U64_PRINTF_ARG(chan->global_identifier), hex_str(chan->identity_digest, DIGEST_LEN)); - /* Clear out its next/prev pointers */ - if (chan->next_with_same_id) { - chan->next_with_same_id->prev_with_same_id = chan->prev_with_same_id; - } - if (chan->prev_with_same_id) { - chan->prev_with_same_id->next_with_same_id = chan->next_with_same_id; - } - chan->next_with_same_id = NULL; - chan->prev_with_same_id = NULL; } }

@@ -698,20 +675,21 @@ channel_find_by_global_id(uint64_t global_identifier) * * This function looks up a channel by the digest of its remote endpoint in * the channel digest map. It's possible that more than one channel to a - * given endpoint exists. Use channel_next_with_digest() and - * channel_prev_with_digest() to walk the list. + * given endpoint exists. Use channel_next_with_digest() to walk the list. */

channel_t * channel_find_by_remote_digest(const char *identity_digest) { channel_t *rv = NULL; + channel_idmap_entry_t *ent, search;

tor_assert(identity_digest);

- /* Search for it in the identity map */ - if (channel_identity_map) { - rv = digestmap_get(channel_identity_map, identity_digest); + memcpy(search.digest, identity_digest, DIGEST_LEN); + ent = HT_FIND(channel_idmap, &channel_identity_map, &search); + if (ent) { + rv = LIST_FIRST(&ent->channel_list); }

return rv; @@ -729,22 +707,7 @@ channel_next_with_digest(channel_t *chan) { tor_assert(chan);

- return chan->next_with_same_id; -} - -/** - * Get previous channel with digest - * - * This function takes a channel and finds the previos channel in the list - * with the same digest. - */ - -channel_t * -channel_prev_with_digest(channel_t *chan) -{ - tor_assert(chan); - - return chan->prev_with_same_id; + return LIST_NEXT(chan, next_with_same_id); }

/** @@ -773,6 +736,9 @@ channel_init(channel_t *chan) SIMPLEQ_INIT(&chan->incoming_queue); SIMPLEQ_INIT(&chan->outgoing_queue);

+ /* Initialize list entries. */ + memset(&chan->next_with_same_id, 0, sizeof(chan->next_with_same_id)); + /* Timestamp it */ channel_timestamp_created(chan); } @@ -2846,13 +2812,10 @@ channel_free_all(void) }

/* Now free channel_identity_map */ - if (channel_identity_map) { - log_debug(LD_CHANNEL, - "Freeing channel_identity_map"); - /* Geez, anything still left over just won't die ... let it leak then */ - digestmap_free(channel_identity_map, NULL); - channel_identity_map = NULL; - } + log_debug(LD_CHANNEL, + "Freeing channel_identity_map"); + /* Geez, anything still left over just won't die ... let it leak then */ + HT_CLEAR(channel_idmap, &channel_identity_map);

log_debug(LD_CHANNEL, "Done cleaning up after channels"); @@ -2959,12 +2922,6 @@ channel_get_for_extend(const char *digest, tor_assert(msg_out); tor_assert(launch_out);

- if (!channel_identity_map) { - *msg_out = "Router not connected (nothing is). Connecting."; - *launch_out = 1; - return NULL; - } - chan = channel_find_by_remote_digest(digest);

/* Walk the list, unrefing the old one and refing the new at each diff --git a/src/or/channel.h b/src/or/channel.h index ccb8fe8..1e7279a 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -122,7 +122,7 @@ struct channel_s { * Linked list of channels with the same identity digest, for the * digest->channel map */ - channel_t *next_with_same_id, *prev_with_same_id; + LIST_ENTRY(channel_s) next_with_same_id;

/* List of incoming cells to handle */ chan_cell_queue_t incoming_queue; @@ -417,9 +417,7 @@ channel_t * channel_find_by_remote_digest(const char *identity_digest);

/** For things returned by channel_find_by_remote_digest(), walk the list. */ - channel_t * channel_next_with_digest(channel_t *chan); -channel_t * channel_prev_with_digest(channel_t *chan);

/* * Metadata queries/updates