Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All hash tables should use tal #5868

Merged
merged 12 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion channeld/full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ struct channel *new_full_channel(const tal_t *ctx,
channel->htlcs = tal(channel, struct htlc_map);
htlc_map_init(channel->htlcs);
memleak_add_helper(channel->htlcs, memleak_help_htlcmap);
tal_add_destructor(channel->htlcs, htlc_map_clear);
}
return channel;
}
Expand Down
30 changes: 15 additions & 15 deletions common/gossmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ struct gossmap {
size_t map_end, map_size;

/* Map of node id -> node */
struct nodeidx_htable nodes;
struct nodeidx_htable *nodes;

/* Map of short_channel_id id -> channel */
struct chanidx_htable channels;
struct chanidx_htable *channels;

/* Array of nodes, so we can use simple index. */
struct gossmap_node *node_arr;
Expand Down Expand Up @@ -235,7 +235,7 @@ static struct node_id nodeidx_id(const ptrint_t *pidx)
struct gossmap_node *gossmap_find_node(const struct gossmap *map,
const struct node_id *id)
{
ptrint_t *pi = nodeidx_htable_get(&map->nodes, *id);
ptrint_t *pi = nodeidx_htable_get(map->nodes, *id);
if (pi)
return ptrint2node(pi);
return NULL;
Expand All @@ -244,7 +244,7 @@ struct gossmap_node *gossmap_find_node(const struct gossmap *map,
struct gossmap_chan *gossmap_find_chan(const struct gossmap *map,
const struct short_channel_id *scid)
{
ptrint_t *pi = chanidx_htable_get(&map->channels, *scid);
ptrint_t *pi = chanidx_htable_get(map->channels, *scid);
if (pi)
return ptrint2chan(pi);
return NULL;
Expand Down Expand Up @@ -295,7 +295,7 @@ static u32 new_node(struct gossmap *map)
static void remove_node(struct gossmap *map, struct gossmap_node *node)
{
u32 nodeidx = gossmap_node_idx(map, node);
if (!nodeidx_htable_del(&map->nodes, node2ptrint(node)))
if (!nodeidx_htable_del(map->nodes, node2ptrint(node)))
abort();
node->nann_off = map->freed_nodes;
free(node->chan_idxs);
Expand Down Expand Up @@ -359,7 +359,7 @@ static struct gossmap_chan *new_channel(struct gossmap *map,
chan->half[1].nodeidx = n2idx;
node_add_channel(map->node_arr + n1idx, gossmap_chan_idx(map, chan));
node_add_channel(map->node_arr + n2idx, gossmap_chan_idx(map, chan));
chanidx_htable_add(&map->channels, chan2ptrint(chan));
chanidx_htable_add(map->channels, chan2ptrint(chan));

return chan;
}
Expand All @@ -386,7 +386,7 @@ static void remove_chan_from_node(struct gossmap *map,
void gossmap_remove_chan(struct gossmap *map, struct gossmap_chan *chan)
{
u32 chanidx = gossmap_chan_idx(map, chan);
if (!chanidx_htable_del(&map->channels, chan2ptrint(chan)))
if (!chanidx_htable_del(map->channels, chan2ptrint(chan)))
abort();
remove_chan_from_node(map, gossmap_nth_node(map, chan, 0), chanidx);
remove_chan_from_node(map, gossmap_nth_node(map, chan, 1), chanidx);
Expand Down Expand Up @@ -460,10 +460,10 @@ static struct gossmap_chan *add_channel(struct gossmap *map,

/* Now we have a channel, we can add nodes to htable */
if (!n[0])
nodeidx_htable_add(&map->nodes,
nodeidx_htable_add(map->nodes,
node2ptrint(map->node_arr + nidx[0]));
if (!n[1])
nodeidx_htable_add(&map->nodes,
nodeidx_htable_add(map->nodes,
node2ptrint(map->node_arr + nidx[1]));

return chan;
Expand Down Expand Up @@ -678,8 +678,10 @@ static bool load_gossip_store(struct gossmap *map, size_t *num_rejected)
* and 10000 nodes, let's assume each channel gets about 750 bytes.
*
* We halve this, since often some records are deleted. */
chanidx_htable_init_sized(&map->channels, map->map_size / 750 / 2);
nodeidx_htable_init_sized(&map->nodes, map->map_size / 2500 / 2);
map->channels = tal(map, struct chanidx_htable);
chanidx_htable_init_sized(map->channels, map->map_size / 750 / 2);
map->nodes = tal(map, struct nodeidx_htable);
nodeidx_htable_init_sized(map->nodes, map->map_size / 2500 / 2);

map->num_chan_arr = map->map_size / 750 / 2 + 1;
map->chan_arr = tal_arr(map, struct gossmap_chan, map->num_chan_arr);
Expand All @@ -697,8 +699,6 @@ static void destroy_map(struct gossmap *map)
{
if (map->mmap)
munmap(map->mmap, map->map_size);
chanidx_htable_clear(&map->channels);
nodeidx_htable_clear(&map->nodes);

for (size_t i = 0; i < tal_count(map->node_arr); i++)
free(map->node_arr[i].chan_idxs);
Expand Down Expand Up @@ -1059,7 +1059,7 @@ struct gossmap_node *gossmap_nth_node(const struct gossmap *map,

size_t gossmap_num_nodes(const struct gossmap *map)
{
return nodeidx_htable_count(&map->nodes);
return nodeidx_htable_count(map->nodes);
}

static struct gossmap_node *node_iter(const struct gossmap *map, size_t start)
Expand All @@ -1084,7 +1084,7 @@ struct gossmap_node *gossmap_next_node(const struct gossmap *map,

size_t gossmap_num_chans(const struct gossmap *map)
{
return chanidx_htable_count(&map->channels);
return chanidx_htable_count(map->channels);
}

static struct gossmap_chan *chan_iter(const struct gossmap *map, size_t start)
Expand Down
5 changes: 4 additions & 1 deletion common/memleak.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ static void children_into_htable(struct htable *memtable, const tal_t *p)
if (streq(name, "tmpctx"))
continue;
}
/* Don't add (resizing!) memtable table! */
if (i == memtable->table)
continue;

htable_add(memtable, hash_ptr(i, NULL), i);
children_into_htable(memtable, i);
}
Expand Down Expand Up @@ -315,7 +319,6 @@ struct htable *memleak_start(const tal_t *ctx)
call_memleak_helpers(memtable, NULL);
}

tal_add_destructor(memtable, htable_clear);
return memtable;
}

Expand Down
13 changes: 13 additions & 0 deletions common/setup.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "config.h"
#include <assert.h>
#include <ccan/ccan/err/err.h>
#include <ccan/ccan/htable/htable.h>
#include <common/autodata.h>
#include <common/setup.h>
#include <common/utils.h>
Expand All @@ -24,6 +25,15 @@ static struct wally_operations wally_tal_ops = {
.free_fn = wally_free,
};

static void *htable_tal(struct htable *ht, size_t len)
{
return tal_arrz(ht, u8, len);
}

static void htable_tal_free(struct htable *ht, void *p)
{
tal_free(p);
}

void common_setup(const char *argv0)
{
Expand All @@ -47,6 +57,9 @@ void common_setup(const char *argv0)
errx(1, "Error setting libwally operations: %i", wally_ret);
secp256k1_ctx = wally_get_secp_context();

/* Make htable* use tal for the tables themselves. */
htable_set_allocator(htable_tal, htable_tal_free);

setup_tmpctx();
}

Expand Down
21 changes: 11 additions & 10 deletions connectd/connectd.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ void destroy_peer(struct peer *peer)
{
assert(!peer->draining);

if (!peer_htable_del(&peer->daemon->peers, peer))
if (!peer_htable_del(peer->daemon->peers, peer))
abort();

/* Tell gossipd to stop asking this peer gossip queries */
Expand Down Expand Up @@ -257,7 +257,7 @@ static struct peer *new_peer(struct daemon *daemon,

/* Now we own it */
tal_steal(peer, peer->to_peer);
peer_htable_add(&daemon->peers, peer);
peer_htable_add(daemon->peers, peer);
tal_add_destructor(peer, destroy_peer);

return peer;
Expand All @@ -282,7 +282,7 @@ struct io_plan *peer_connected(struct io_conn *conn,
bool option_gossip_queries;

/* We remove any previous connection immediately, on the assumption it's dead */
peer = peer_htable_get(&daemon->peers, id);
peer = peer_htable_get(daemon->peers, id);
if (peer)
tal_free(peer);

Expand Down Expand Up @@ -1724,7 +1724,7 @@ static void try_connect_peer(struct daemon *daemon,
struct connecting *connect;

/* Already existing? Must have crossed over, it'll know soon. */
if (peer_htable_get(&daemon->peers, id))
if (peer_htable_get(daemon->peers, id))
return;

/* If we're trying to connect it right now, that's OK. */
Expand Down Expand Up @@ -1827,7 +1827,7 @@ static void peer_discard(struct daemon *daemon, const u8 *msg)

/* We should stay in sync with lightningd, but this can happen
* under stress. */
peer = peer_htable_get(&daemon->peers, &id);
peer = peer_htable_get(daemon->peers, &id);
if (!peer)
return;
/* If it's reconnected already, it will learn soon. */
Expand All @@ -1852,7 +1852,7 @@ static void peer_final_msg(struct io_conn *conn,

/* This can happen if peer hung up on us (or wrong counter
* if it reconnected). */
peer = peer_htable_get(&daemon->peers, &id);
peer = peer_htable_get(daemon->peers, &id);
if (peer && peer->counter == counter)
multiplex_final_msg(peer, take(finalmsg));
}
Expand All @@ -1868,7 +1868,7 @@ static void dev_connect_memleak(struct daemon *daemon, const u8 *msg)

/* Now delete daemon and those which it has pointers to. */
memleak_scan_obj(memtable, daemon);
memleak_scan_htable(memtable, &daemon->peers.raw);
memleak_scan_htable(memtable, &daemon->peers->raw);

found_leak = dump_memleak(memtable, memleak_status_broken);
daemon_conn_send(daemon->master,
Expand Down Expand Up @@ -1995,7 +1995,7 @@ static struct io_plan *recv_gossip(struct io_conn *conn,
status_failed(STATUS_FAIL_GOSSIP_IO, "Unknown msg %i",
fromwire_peektype(msg));

peer = peer_htable_get(&daemon->peers, &dst);
peer = peer_htable_get(daemon->peers, &dst);
if (peer)
inject_peer_msg(peer, take(gossip_msg));

Expand All @@ -2007,7 +2007,7 @@ static struct io_plan *recv_gossip(struct io_conn *conn,
#if DEVELOPER
static void memleak_daemon_cb(struct htable *memtable, struct daemon *daemon)
{
memleak_scan_htable(memtable, &daemon->peers.raw);
memleak_scan_htable(memtable, &daemon->peers->raw);
}
#endif /* DEVELOPER */

Expand All @@ -2023,7 +2023,8 @@ int main(int argc, char *argv[])
/* Allocate and set up our simple top-level structure. */
daemon = tal(NULL, struct daemon);
daemon->connection_counter = 1;
peer_htable_init(&daemon->peers);
daemon->peers = tal(daemon, struct peer_htable);
peer_htable_init(daemon->peers);
memleak_add_helper(daemon, memleak_daemon_cb);
list_head_init(&daemon->connecting);
timers_init(&daemon->timers, time_mono());
Expand Down
2 changes: 1 addition & 1 deletion connectd/connectd.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ struct daemon {

/* Peers that we've handed to `lightningd`, which it hasn't told us
* have disconnected. */
struct peer_htable peers;
struct peer_htable *peers;

/* Peers we are trying to reach */
struct list_head connecting;
Expand Down
6 changes: 0 additions & 6 deletions connectd/gossip_rcvd_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,11 @@ static size_t rehash(const void *key, void *unused)
return ptr2int(key);
}

static void destroy_msg_map(struct htable *ht)
{
htable_clear(ht);
}

static struct htable *new_msg_map(const tal_t *ctx)
{
struct htable *ht = tal(ctx, struct htable);

htable_init(ht, rehash, NULL);
tal_add_destructor(ht, destroy_msg_map);
return ht;
}

Expand Down
6 changes: 3 additions & 3 deletions connectd/multiplex.c
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ void send_custommsg(struct daemon *daemon, const u8 *msg)
master_badmsg(WIRE_CONNECTD_CUSTOMMSG_OUT, msg);

/* Races can happen: this might be gone by now. */
peer = peer_htable_get(&daemon->peers, &id);
peer = peer_htable_get(daemon->peers, &id);
if (peer)
inject_peer_msg(peer, take(custommsg));
}
Expand Down Expand Up @@ -1242,7 +1242,7 @@ void peer_connect_subd(struct daemon *daemon, const u8 *msg, int fd)
master_badmsg(WIRE_CONNECTD_PEER_CONNECT_SUBD, msg);

/* Races can happen: this might be gone by now (or reconnected!). */
peer = peer_htable_get(&daemon->peers, &id);
peer = peer_htable_get(daemon->peers, &id);
if (!peer || peer->counter != counter) {
close(fd);
return;
Expand Down Expand Up @@ -1276,7 +1276,7 @@ void send_manual_ping(struct daemon *daemon, const u8 *msg)
if (!fromwire_connectd_ping(msg, &id, &num_pong_bytes, &len))
master_badmsg(WIRE_CONNECTD_PING, msg);

peer = peer_htable_get(&daemon->peers, &id);
peer = peer_htable_get(daemon->peers, &id);
if (!peer) {
daemon_conn_send(daemon->master,
take(towire_connectd_ping_reply(NULL,
Expand Down
4 changes: 2 additions & 2 deletions connectd/onion_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void onionmsg_req(struct daemon *daemon, const u8 *msg)

/* Even though lightningd checks for valid ids, there's a race
* where it might vanish before we read this command. */
peer = peer_htable_get(&daemon->peers, &id);
peer = peer_htable_get(daemon->peers, &id);
if (peer) {
u8 *omsg = towire_onion_message(NULL, &blinding, onionmsg);
inject_peer_msg(peer, take(omsg));
Expand Down Expand Up @@ -86,7 +86,7 @@ void handle_onion_message(struct daemon *daemon,

/* FIXME: Handle short_channel_id! */
node_id_from_pubkey(&next_node_id, &next_node);
next_peer = peer_htable_get(&daemon->peers, &next_node_id);
next_peer = peer_htable_get(daemon->peers, &next_node_id);
if (!next_peer) {
status_peer_debug(&peer->id,
"onion msg: unknown next peer %s",
Expand Down
11 changes: 8 additions & 3 deletions connectd/test/run-gossip_rcvd_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,14 @@ int main(int argc, char *argv[])
assert(htable_count(f->cur) == 0);
assert(htable_count(f->old) == 0);

/* They should have no children, and f should only have 2. */
assert(!tal_first(f->cur));
assert(!tal_first(f->old));
/* They should have no children (except htable contents for one!), and
* f should only have 2. */
if (tal_first(f->cur) == NULL)
assert(tal_first(f->old) == f->old->table);
else {
assert(tal_first(f->cur) == f->cur->table);
assert(tal_first(f->old) == NULL);
}

assert((tal_first(f) == f->cur
&& tal_next(f->cur) == f->old
Expand Down
6 changes: 0 additions & 6 deletions gossipd/gossip_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -407,11 +407,6 @@ static void move_broadcast(struct offmap *offmap,
offmap_del(offmap, omap);
}

static void destroy_offmap(struct offmap *offmap)
{
offmap_clear(offmap);
}

/**
* Rewrite the on-disk gossip store, compacting it along the way
*
Expand Down Expand Up @@ -453,7 +448,6 @@ bool gossip_store_compact(struct gossip_store *gs)
/* Walk old file, copy everything and remember new offsets. */
offmap = tal(tmpctx, struct offmap);
offmap_init_sized(offmap, gs->count);
tal_add_destructor(offmap, destroy_offmap);

/* Start by writing all channel announcements and updates. */
off = 1;
Expand Down
Loading