From 7ad75b768b82742188f425fba5ba310b2f41930d Mon Sep 17 00:00:00 2001 From: bizhiyuan Date: Thu, 17 Oct 2024 20:40:34 +0800 Subject: [PATCH] Apply find_* function refactorings --- ...ee-argument-versions-of-FOREACH_-NOD.patch | 38 ++ ...lobal-booth_conf-variable-in-find_id.patch | 544 ++++++++++++++++++ ...l-booth_conf-variable-in-find_myself.patch | 154 +++++ ...bal-booth_conf-variable-in-find_name.patch | 189 ++++++ ...Refactor-Rename-FOREACH_-NODE-TICKET.patch | 240 ++++++++ booth.spec | 10 +- 6 files changed, 1174 insertions(+), 1 deletion(-) create mode 100644 backport-Refactor-Add-three-argument-versions-of-FOREACH_-NOD.patch create mode 100644 backport-Refactor-Remove-global-booth_conf-variable-in-find_id.patch create mode 100644 backport-Refactor-Remove-global-booth_conf-variable-in-find_myself.patch create mode 100644 backport-Refactor-Remove-global-booth_conf-variable-in-find_name.patch create mode 100644 backport-Refactor-Rename-FOREACH_-NODE-TICKET.patch diff --git a/backport-Refactor-Add-three-argument-versions-of-FOREACH_-NOD.patch b/backport-Refactor-Add-three-argument-versions-of-FOREACH_-NOD.patch new file mode 100644 index 0000000..ded85aa --- /dev/null +++ b/backport-Refactor-Add-three-argument-versions-of-FOREACH_-NOD.patch @@ -0,0 +1,38 @@ +From e3dccd88d90f0b822ed815cb9a43e84c961a92f9 Mon Sep 17 00:00:00 2001 +From: Chris Lumens +Date: Thu, 11 Jul 2024 10:33:02 -0400 +Subject: [PATCH 2/5] Refactor: Add three argument versions of + FOREACH_{NODE,TICKET}. + +These take an additional argument that is the config object. Nothing +uses these at the moment - I'm going to move callers from the old +renamed versions to these over a bunch of commits. +--- + src/ticket.h | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +diff --git a/src/ticket.h b/src/ticket.h +index af4b195..dc6869e 100644 +--- a/src/ticket.h ++++ b/src/ticket.h +@@ -35,6 +35,17 @@ extern int TIME_RES; + #define DEFAULT_RETRIES 10 + + ++#define FOREACH_TICKET(b_, i_, t_) \ ++ for (i_ = 0; \ ++ (t_ = (b_)->ticket + i_, i_ < (b_)->ticket_count); \ ++ i_++) ++ ++#define FOREACH_NODE(b_, i_, n_) \ ++ for (i_ = 0; \ ++ (n_ = (b_)->site + i_, i_ < (b_)->site_count); \ ++ i_++) ++ ++ + #define _FOREACH_TICKET(i_, t_) \ + for (i_ = 0; \ + (t_ = booth_conf->ticket + i_, i_ < booth_conf->ticket_count); \ +-- +2.25.1 + diff --git a/backport-Refactor-Remove-global-booth_conf-variable-in-find_id.patch b/backport-Refactor-Remove-global-booth_conf-variable-in-find_id.patch new file mode 100644 index 0000000..c068880 --- /dev/null +++ b/backport-Refactor-Remove-global-booth_conf-variable-in-find_id.patch @@ -0,0 +1,544 @@ +From f0b15ec9a634ec51c69f8dce944c7652baed5a15 Mon Sep 17 00:00:00 2001 +From: Chris Lumens +Date: Thu, 11 Jul 2024 11:37:02 -0400 +Subject: [PATCH 4/5] Refactor: Remove global booth_conf variable in + find_site_by_id... +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +...as well as functions that call it. + +Co-authored-by: Jan Pokorný +--- + src/booth.h | 8 ++++++-- + src/config.c | 9 +++++---- + src/config.h | 25 ++++++++++++++++++++++++- + src/main.c | 17 +++++++++-------- + src/manual.c | 2 +- + src/pacemaker.c | 41 ++++++++++++++++++++++------------------- + src/pacemaker.h | 2 +- + src/raft.c | 2 +- + src/ticket.c | 10 +++++----- + src/ticket.h | 25 +++++++++++++++++++++++-- + src/transport.c | 16 ++++++++-------- + src/transport.h | 12 +++++++++++- + 12 files changed, 116 insertions(+), 53 deletions(-) + +diff --git a/src/booth.h b/src/booth.h +index 0cd43c0..799cdf9 100644 +--- a/src/booth.h ++++ b/src/booth.h +@@ -77,6 +77,10 @@ + /* Says that another one should recover. */ + #define TICKET_LOST CHAR2CONST('L', 'O', 'S', 'T') + ++struct booth_config; ++ ++typedef void (*workfn_t)(struct booth_config *conf, int ci); ++ + + typedef char boothc_site[BOOTH_NAME_LEN]; + typedef char boothc_ticket[BOOTH_NAME_LEN]; +@@ -338,7 +342,7 @@ struct client { + const struct booth_transport *transport; + struct boothc_ticket_msg *msg; + int offset; /* bytes read so far into msg */ +- void (*workfn)(int); ++ workfn_t workfn; + void (*deadfn)(int); + }; + +@@ -347,7 +351,7 @@ extern struct pollfd *pollfds; + + + int client_add(int fd, const struct booth_transport *tpt, +- void (*workfn)(int ci), void (*deadfn)(int ci)); ++ workfn_t workfn, void (*deadfn)(int ci)); + int find_client_by_fd(int fd); + void safe_copy(char *dest, char *value, size_t buflen, const char *description); + int update_authkey(void); +diff --git a/src/config.c b/src/config.c +index 5236505..4eafc35 100644 +--- a/src/config.c ++++ b/src/config.c +@@ -1041,7 +1041,8 @@ int find_site_by_name(struct booth_config *conf, const char *site, + return 0; + } + +-int find_site_by_id(uint32_t site_id, struct booth_site **node) ++int find_site_by_id(struct booth_config *conf, ++ uint32_t site_id, struct booth_site **node) + { + struct booth_site *n; + int i; +@@ -1051,10 +1052,11 @@ int find_site_by_id(uint32_t site_id, struct booth_site **node) + return 1; + } + +- if (!booth_conf) ++ if (conf == NULL) { + return 0; ++ } + +- _FOREACH_NODE(i, n) { ++ FOREACH_NODE(conf, i, n) { + if (n->site_id == site_id) { + *node = n; + return 1; +@@ -1064,7 +1066,6 @@ int find_site_by_id(uint32_t site_id, struct booth_site **node) + return 0; + } + +- + const char *type_to_string(int type) + { + switch (type) +diff --git a/src/config.h b/src/config.h +index 1a7f40c..fbe90d1 100644 +--- a/src/config.h ++++ b/src/config.h +@@ -25,6 +25,17 @@ + #include "booth.h" + #include "timer.h" + #include "raft.h" ++ ++/* Forward declaration of the booth config structure that will be defined later. ++ * This is necessary here because transport.h references booth_config, but this ++ * file also references transport_layer_t. We need some way to break the ++ * circular dependency. ++ * ++ * This also means that config.h must always be included before transport.h in ++ * any source files. ++ */ ++struct booth_config; ++ + #include "transport.h" + + +@@ -369,7 +380,19 @@ int check_config(struct booth_config *conf, int type); + int find_site_by_name(struct booth_config *conf, const char *site, + struct booth_site **node, int any_type); + +-int find_site_by_id(uint32_t site_id, struct booth_site **node); ++ ++/** ++ * @internal ++ * Find site in booth configuration by a hash (id) ++ * ++ * @param[in,out] conf config object to refer to ++ * @param[in] site_id hash (id) to match against previously resolved ones ++ * @param[out] node relevant tracked data when found ++ * ++ * @return 0 if nothing found, or 1 when found (node assigned accordingly) ++ */ ++int find_site_by_id(struct booth_config *conf, uint32_t site_id, ++ struct booth_site **node); + + const char *type_to_string(int type); + +diff --git a/src/main.c b/src/main.c +index 445655c..609dcdf 100644 +--- a/src/main.c ++++ b/src/main.c +@@ -165,8 +165,7 @@ static void client_dead(int ci) + } + + int client_add(int fd, const struct booth_transport *tpt, +- void (*workfn)(int ci), +- void (*deadfn)(int ci)) ++ workfn_t workfn, void (*deadfn)(int ci)) + { + int i; + struct client *c; +@@ -521,7 +520,7 @@ static int process_signals(void) + + static int loop(int fd) + { +- void (*workfn) (int ci); ++ workfn_t workfn; + void (*deadfn) (int ci); + int rv, i; + +@@ -529,9 +528,10 @@ static int loop(int fd) + if (rv < 0) + goto fail; + +- rv = setup_ticket(); +- if (rv < 0) ++ rv = setup_ticket(booth_conf); ++ if (rv < 0) { + goto fail; ++ } + + rv = write_daemon_state(fd, BOOTHD_STARTED); + if (rv != 0) { +@@ -559,8 +559,9 @@ static int loop(int fd) + + if (pollfds[i].revents & POLLIN) { + workfn = clients[i].workfn; +- if (workfn) +- workfn(i); ++ if (workfn) { ++ workfn(booth_conf, i); ++ } + } + if (pollfds[i].revents & + (POLLERR | POLLHUP | POLLNVAL)) { +@@ -837,7 +838,7 @@ read_more: + if (rv == 1) { + tpt->close(site); + leader_id = ntohl(reply.ticket.leader); +- if (!find_site_by_id(leader_id, &site)) { ++ if (!find_site_by_id(booth_conf, leader_id, &site)) { + log_error("Message with unknown redirect site %x received", leader_id); + rv = -1; + goto out_close; +diff --git a/src/manual.c b/src/manual.c +index ee9e858..5fc5eeb 100644 +--- a/src/manual.c ++++ b/src/manual.c +@@ -18,9 +18,9 @@ + + #include "manual.h" + ++#include "config.h" + #include "transport.h" + #include "ticket.h" +-#include "config.h" + #include "log.h" + #include "request.h" + +diff --git a/src/pacemaker.c b/src/pacemaker.c +index 084dd4c..9febb08 100644 +--- a/src/pacemaker.c ++++ b/src/pacemaker.c +@@ -151,8 +151,8 @@ static int pcmk_del_attr(struct ticket_config *tk, const char *attr) + } + + +-typedef int (*attr_f)(struct ticket_config *tk, const char *name, +- const char *val); ++typedef int (*attr_f)(struct booth_config *conf, struct ticket_config *tk, ++ const char *name, const char *val); + + struct attr_tab + { +@@ -160,15 +160,15 @@ struct attr_tab + attr_f handling_f; + }; + +-static int save_expires(struct ticket_config *tk, const char *name, +- const char *val) ++static int save_expires(struct booth_config *conf, struct ticket_config *tk, ++ const char *name, const char *val) + { + secs2tv(unwall_ts(atol(val)), &tk->term_expires); + return 0; + } + +-static int save_term(struct ticket_config *tk, const char *name, +- const char *val) ++static int save_term(struct booth_config *conf, struct ticket_config *tk, ++ const char *name, const char *val) + { + tk->current_term = atol(val); + return 0; +@@ -188,23 +188,23 @@ static int parse_boolean(const char *val) + return v; + } + +-static int save_granted(struct ticket_config *tk, const char *name, +- const char *val) ++static int save_granted(struct booth_config *conf, struct ticket_config *tk, ++ const char *name, const char *val) + { + tk->is_granted = parse_boolean(val); + return 0; + } + +-static int save_owner(struct ticket_config *tk, const char *name, +- const char *val) ++static int save_owner(struct booth_config *conf, struct ticket_config *tk, ++ const char *name, const char *val) + { + /* No check, node could have been deconfigured. */ + tk->leader = NULL; +- return !find_site_by_id(atol(val), &tk->leader); ++ return !find_site_by_id(conf, atol(val), &tk->leader); + } + +-static int ignore_attr(struct ticket_config *tk, const char *name, +- const char *val) ++static int ignore_attr(struct booth_config *conf, struct ticket_config *tk, ++ const char *name, const char *val) + { + return 0; + } +@@ -276,7 +276,8 @@ out: + return rv | pipe_rv; + } + +-static int save_attributes(struct ticket_config *tk, xmlDocPtr doc) ++static int save_attributes(struct booth_config *conf, struct ticket_config *tk, ++ xmlDocPtr doc) + { + int rv = 0, rc; + xmlNodePtr n; +@@ -297,7 +298,8 @@ static int save_attributes(struct ticket_config *tk, xmlDocPtr doc) + v = xmlGetProp(n, attr->name); + for (atp = attr_handlers; atp->name; atp++) { + if (!strcmp(atp->name, (const char *) attr->name)) { +- rc = atp->handling_f(tk, (const char *) attr->name, ++ rc = atp->handling_f(conf, tk, ++ (const char *) attr->name, + (const char *) v); + break; + } +@@ -318,7 +320,8 @@ static int save_attributes(struct ticket_config *tk, xmlDocPtr doc) + + #define CHUNK_SIZE 256 + +-static int parse_ticket_state(struct ticket_config *tk, FILE *p) ++static int parse_ticket_state(struct booth_config *conf, struct ticket_config *tk, ++ FILE *p) + { + int rv = 0; + GString *input = NULL; +@@ -359,7 +362,7 @@ static int parse_ticket_state(struct ticket_config *tk, FILE *p) + rv = -EINVAL; + goto out; + } +- rv = save_attributes(tk, doc); ++ rv = save_attributes(conf, tk, doc); + + out: + if (doc) +@@ -369,7 +372,7 @@ out: + return rv; + } + +-static int pcmk_load_ticket(struct ticket_config *tk) ++static int pcmk_load_ticket(struct booth_config *conf, struct ticket_config *tk) + { + char cmd[COMMAND_MAX]; + int rv = 0, pipe_rv; +@@ -393,7 +396,7 @@ static int pcmk_load_ticket(struct ticket_config *tk) + return (pipe_rv != 0 ? pipe_rv : EINVAL); + } + +- rv = parse_ticket_state(tk, p); ++ rv = parse_ticket_state(conf, tk, p); + + if (!tk->leader) { + /* Hmm, no site found for the ticket we have in the +diff --git a/src/pacemaker.h b/src/pacemaker.h +index 3a81724..412fe6f 100644 +--- a/src/pacemaker.h ++++ b/src/pacemaker.h +@@ -26,7 +26,7 @@ + struct ticket_handler { + int (*grant_ticket) (struct ticket_config *tk); + int (*revoke_ticket) (struct ticket_config *tk); +- int (*load_ticket) (struct ticket_config *tk); ++ int (*load_ticket) (struct booth_config *conf, struct ticket_config *tk); + int (*set_attr) (struct ticket_config *tk, const char *a, const char *v); + int (*get_attr) (struct ticket_config *tk, const char *a, const char **vp); + int (*del_attr) (struct ticket_config *tk, const char *a); +diff --git a/src/raft.c b/src/raft.c +index 1c73b51..ec0cf99 100644 +--- a/src/raft.c ++++ b/src/raft.c +@@ -23,9 +23,9 @@ + #include + #include "booth.h" + #include "timer.h" ++#include "config.h" + #include "transport.h" + #include "inline-fn.h" +-#include "config.h" + #include "raft.h" + #include "ticket.h" + #include "request.h" +diff --git a/src/ticket.c b/src/ticket.c +index 384a229..353f080 100644 +--- a/src/ticket.c ++++ b/src/ticket.c +@@ -612,16 +612,16 @@ void update_ticket_state(struct ticket_config *tk, struct booth_site *sender) + } + } + +-int setup_ticket(void) ++int setup_ticket(struct booth_config *conf) + { + struct ticket_config *tk; + int i; + +- _FOREACH_TICKET(i, tk) { ++ FOREACH_TICKET(conf, i, tk) { + reset_ticket(tk); + + if (local->type == SITE) { +- if (!pcmk_handler.load_ticket(tk)) { ++ if (!pcmk_handler.load_ticket(conf, tk)) { + update_ticket_state(tk, NULL); + } + tk->update_cib = 1; +@@ -1197,7 +1197,7 @@ static void update_acks( + } + + /* read ticket message */ +-int ticket_recv(void *buf, struct booth_site *source) ++int ticket_recv(struct booth_config *conf, void *buf, struct booth_site *source) + { + struct boothc_ticket_msg *msg; + struct ticket_config *tk; +@@ -1215,7 +1215,7 @@ int ticket_recv(void *buf, struct booth_site *source) + + + leader_u = ntohl(msg->ticket.leader); +- if (!find_site_by_id(leader_u, &leader)) { ++ if (!find_site_by_id(conf, leader_u, &leader)) { + tk_log_error("message with unknown leader %u received", leader_u); + source->invalid_cnt++; + return -EINVAL; +diff --git a/src/ticket.h b/src/ticket.h +index dc6869e..f28ac6e 100644 +--- a/src/ticket.h ++++ b/src/ticket.h +@@ -112,11 +112,32 @@ int grant_ticket(struct ticket_config *ticket); + int revoke_ticket(struct ticket_config *ticket); + int list_ticket(char **pdata, unsigned int *len); + +-int ticket_recv(void *buf, struct booth_site *source); ++/** ++ * @internal ++ * Second stage of incoming datagram handling (after authentication) ++ * ++ * @param[in,out] conf config object to refer to ++ * @param[in] buf raw message to act upon ++ * @param[in] source member originating this message ++ * ++ * @return 0 on success or negative value (-1 or -errno) on error ++ */ ++int ticket_recv(struct booth_config *conf, void *buf, struct booth_site *source); ++ + void reset_ticket(struct ticket_config *tk); + void reset_ticket_and_set_no_leader(struct ticket_config *tk); + void update_ticket_state(struct ticket_config *tk, struct booth_site *sender); +-int setup_ticket(void); ++ ++/** ++ * @internal ++ * Broadcast the initial state query ++ * ++ * @param[in,out] conf config object to use as a starting point ++ * ++ * @return 0 (for the time being) ++ */ ++int setup_ticket(struct booth_config *conf); ++ + int check_max_len_valid(const char *s, int max); + + int do_grant_ticket(struct ticket_config *ticket, int options); +diff --git a/src/transport.c b/src/transport.c +index 4fb754e..56e5108 100644 +--- a/src/transport.c ++++ b/src/transport.c +@@ -59,7 +59,7 @@ struct booth_site *local = NULL; + * or positive integer to indicate sender's ID that will then be + * emitted in the error log message together with the real source + * address if this is available */ +-static int (*deliver_fn) (void *msg, int msglen); ++static int (*deliver_fn) (struct booth_config *conf, void *msg, int msglen); + + + static void parse_rtattr(struct rtattr *tb[], +@@ -431,7 +431,7 @@ int read_client(struct client *req_cl) + + + /* Only used for client requests (tcp) */ +-static void process_connection(int ci) ++static void process_connection(struct booth_config *conf, int ci) + { + struct client *req_cl; + void *msg = NULL; +@@ -507,7 +507,7 @@ kill: + } + + +-static void process_tcp_listener(int ci) ++static void process_tcp_listener(struct booth_config *conf, int ci) + { + int fd, i, flags, one = 1; + socklen_t addrlen = sizeof(struct sockaddr); +@@ -803,7 +803,7 @@ ex: + + + /* Receive/process callback for UDP */ +-static void process_recv(int ci) ++static void process_recv(struct booth_config *conf, int ci) + { + struct sockaddr_storage sa; + int rv; +@@ -824,7 +824,7 @@ static void process_recv(int ci) + if (rv == -1) + return; + +- rv = deliver_fn((void*)msg, rv); ++ rv = deliver_fn(conf, (void*) msg, rv); + if (rv > 0) { + if (getnameinfo((struct sockaddr *)&sa, sa_len, + buffer, sizeof(buffer), NULL, 0, +@@ -1105,7 +1105,7 @@ int send_header_plus(int fd, struct boothc_hdr_msg *msg, void *data, int len) + } + + /* UDP message receiver (see also deliver_fn declaration's comment) */ +-int message_recv(void *msg, int msglen) ++int message_recv(struct booth_config *conf, void *msg, int msglen) + { + uint32_t from; + struct boothc_header *header; +@@ -1114,7 +1114,7 @@ int message_recv(void *msg, int msglen) + header = (struct boothc_header *)msg; + + from = ntohl(header->from); +- if (!find_site_by_id(from, &source)) { ++ if (!find_site_by_id(conf, from, &source)) { + /* caller knows the actual source address, pass + the (assuredly) positive number and let it report */ + from = from ? from : ~from; /* avoid 0 (success) */ +@@ -1142,6 +1142,6 @@ int message_recv(void *msg, int msglen) + */ + return attr_recv(msg, source); + } else { +- return ticket_recv(msg, source); ++ return ticket_recv(conf, msg, source); + } + } +diff --git a/src/transport.h b/src/transport.h +index 41e488b..e04d0ef 100644 +--- a/src/transport.h ++++ b/src/transport.h +@@ -70,7 +70,17 @@ int booth_udp_send_auth(struct booth_site *to, void *buf, int len); + int booth_tcp_open(struct booth_site *to); + int booth_tcp_send(struct booth_site *to, void *buf, int len); + +-int message_recv(void *msg, int msglen); ++/** ++ * @internal ++ * First stage of incoming datagram handling (authentication) ++ * ++ * @param[in,out] conf config object to refer to ++ * @param[in] msg raw message to act upon ++ * @param[in] msglen length of #msg ++ * ++ * @return 0 on success or negative value (-1 or -errno) on error ++ */ ++int message_recv(struct booth_config *conf, void *msg, int msglen); + + inline static void * node_to_addr_pointer(struct booth_site *node) { + switch (node->family) { +-- +2.25.1 + diff --git a/backport-Refactor-Remove-global-booth_conf-variable-in-find_myself.patch b/backport-Refactor-Remove-global-booth_conf-variable-in-find_myself.patch new file mode 100644 index 0000000..41abd65 --- /dev/null +++ b/backport-Refactor-Remove-global-booth_conf-variable-in-find_myself.patch @@ -0,0 +1,154 @@ +From 0e549bc94256119334a5f949cff2fef28b6d14f7 Mon Sep 17 00:00:00 2001 +From: Chris Lumens +Date: Thu, 11 Jul 2024 11:59:27 -0400 +Subject: [PATCH 5/5] Refactor: Remove global booth_conf variable in + find_myself... +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +...as well as functions that call it. Also, mark _find_myself as static +and remove the unnecessary function prototype for it. + +Co-authored-by: Jan Pokorný +--- + src/main.c | 6 +++--- + src/transport.c | 37 ++++++++++++++++++++----------------- + src/transport.h | 14 +++++++++++++- + 3 files changed, 36 insertions(+), 21 deletions(-) + +diff --git a/src/main.c b/src/main.c +index 609dcdf..c6f6bb6 100644 +--- a/src/main.c ++++ b/src/main.c +@@ -399,9 +399,9 @@ static int setup_config(struct booth_config **conf, int type) + return -EINVAL; + } + local->local = 1; +- } else +- find_myself(NULL, type == CLIENT || type == GEOSTORE); +- ++ } else { ++ find_myself(booth_conf, NULL, type == CLIENT || type == GEOSTORE); ++ } + + rv = check_config(booth_conf, type); + if (rv < 0) +diff --git a/src/transport.c b/src/transport.c +index 56e5108..791029c 100644 +--- a/src/transport.c ++++ b/src/transport.c +@@ -78,11 +78,12 @@ enum match_type { + EXACT_MATCH, + }; + +-static int find_address(unsigned char ipaddr[BOOTH_IPADDR_LEN], +- int family, int prefixlen, +- int fuzzy_allowed, +- struct booth_site **me, +- int *address_bits_matched) ++static int find_address(struct booth_config *conf, ++ unsigned char ipaddr[BOOTH_IPADDR_LEN], ++ int family, int prefixlen, ++ int fuzzy_allowed, ++ struct booth_site **me, ++ int *address_bits_matched) + { + int i; + struct booth_site *node; +@@ -92,13 +93,14 @@ static int find_address(unsigned char ipaddr[BOOTH_IPADDR_LEN], + int matched; + enum match_type did_match = NO_MATCH; + ++ assert(conf != NULL); + + bytes = prefixlen / 8; + bits_left = prefixlen % 8; + /* One bit left to check means ignore 7 lowest bits. */ + mask = ~( (1 << (8 - bits_left)) -1); + +- _FOREACH_NODE(i, node) { ++ FOREACH_NODE(conf, i, node) { + if (family != node->family) + continue; + n_a = node_to_addr_pointer(node); +@@ -140,8 +142,8 @@ static int find_address(unsigned char ipaddr[BOOTH_IPADDR_LEN], + } + + +-int _find_myself(int family, struct booth_site **mep, int fuzzy_allowed); +-int _find_myself(int family, struct booth_site **mep, int fuzzy_allowed) ++static int _find_myself(struct booth_config *conf, int family, ++ struct booth_site **mep, int fuzzy_allowed) + { + int fd; + struct sockaddr_nl nladdr; +@@ -247,9 +249,9 @@ int _find_myself(int family, struct booth_site **mep, int fuzzy_allowed) + * the function find_address will try to return another, most similar + * address (with the longest possible number of same bytes). */ + if (ifa->ifa_prefixlen > address_bits_matched) { +- find_address(ipaddr, +- ifa->ifa_family, ifa->ifa_prefixlen, +- fuzzy_allowed, &me, &address_bits_matched); ++ find_address(conf, ipaddr, ++ ifa->ifa_family, ifa->ifa_prefixlen, ++ fuzzy_allowed, &me, &address_bits_matched); + + if (me) { + log_debug("found myself at %s (%d bits matched)", +@@ -263,9 +265,9 @@ int _find_myself(int family, struct booth_site **mep, int fuzzy_allowed) + * call the function find_address with disabled searching of + * similar addresses (fuzzy_allowed == 0) */ + else if (ifa->ifa_prefixlen == address_bits_matched) { +- find_address(ipaddr, +- ifa->ifa_family, ifa->ifa_prefixlen, +- 0 /* fuzzy_allowed */, &me, &address_bits_matched); ++ find_address(conf, ipaddr, ++ ifa->ifa_family, ifa->ifa_prefixlen, ++ 0 /* fuzzy_allowed */, &me, &address_bits_matched); + + if (me) { + log_debug("found myself at %s (exact match)", +@@ -290,10 +292,11 @@ found: + return 1; + } + +-int find_myself(struct booth_site **mep, int fuzzy_allowed) ++int find_myself(struct booth_config *conf, struct booth_site **mep, ++ int fuzzy_allowed) + { +- return _find_myself(AF_INET6, mep, fuzzy_allowed) || +- _find_myself(AF_INET, mep, fuzzy_allowed); ++ return _find_myself(conf, AF_INET6, mep, fuzzy_allowed) || ++ _find_myself(conf, AF_INET, mep, fuzzy_allowed); + } + + +diff --git a/src/transport.h b/src/transport.h +index e04d0ef..4f4d497 100644 +--- a/src/transport.h ++++ b/src/transport.h +@@ -58,7 +58,19 @@ struct booth_transport { + }; + + extern const struct booth_transport booth_transport[TRANSPORT_ENTRIES]; +-int find_myself(struct booth_site **me, int fuzzy_allowed); ++ ++/** ++ * @internal ++ * Attempts to pick identity of self from config-tracked enumeration of sites ++ * ++ * @param[in,out] conf config object to refer to ++ * @param[out] mep when self-discovery successful, site pointer is stored here ++ * @param[in] fuzzy_allowed whether it's OK to approximate the match ++ * ++ * @return 0 on success or negative value (-1 or -errno) on error ++ */ ++int find_myself(struct booth_config *conf, struct booth_site **me, ++ int fuzzy_allowed); + + int read_client(struct client *req_cl); + int check_boothc_header(struct boothc_header *data, int len_incl_data); +-- +2.25.1 + diff --git a/backport-Refactor-Remove-global-booth_conf-variable-in-find_name.patch b/backport-Refactor-Remove-global-booth_conf-variable-in-find_name.patch new file mode 100644 index 0000000..abf5865 --- /dev/null +++ b/backport-Refactor-Remove-global-booth_conf-variable-in-find_name.patch @@ -0,0 +1,189 @@ +From a6bbee968ec9a2a4c11b7f1a90b8a21dfc07d2e7 Mon Sep 17 00:00:00 2001 +From: Chris Lumens +Date: Thu, 11 Jul 2024 10:45:35 -0400 +Subject: [PATCH 3/5] Refactor: Remove global booth_conf variable in + find_site_by_name... +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +...as well as functions that call it. Also, change the site argument to +be const. + +Co-authored-by: Jan Pokorný +--- + src/attr.c | 4 ++-- + src/attr.h | 13 ++++++++++++- + src/config.c | 21 +++++++++++++-------- + src/config.h | 15 ++++++++++++++- + src/main.c | 8 ++++---- + 5 files changed, 45 insertions(+), 16 deletions(-) + +diff --git a/src/attr.c b/src/attr.c +index daa7648..086dc98 100644 +--- a/src/attr.c ++++ b/src/attr.c +@@ -149,7 +149,7 @@ static int read_server_reply( + return rv; + } + +-int do_attr_command(cmd_request_t cmd) ++int do_attr_command(struct booth_config *conf, cmd_request_t cmd) + { + struct booth_site *site = NULL; + struct boothc_header *header; +@@ -160,7 +160,7 @@ int do_attr_command(cmd_request_t cmd) + if (!*cl.site) + site = local; + else { +- if (!find_site_by_name(cl.site, &site, 1)) { ++ if (!find_site_by_name(conf, cl.site, &site, 1)) { + log_error("Site \"%s\" not configured.", cl.site); + goto out_close; + } +diff --git a/src/attr.h b/src/attr.h +index 1c680bd..547d19a 100644 +--- a/src/attr.h ++++ b/src/attr.h +@@ -31,7 +31,18 @@ + + void print_geostore_usage(void); + int test_attr_reply(cmd_result_t reply_code, cmd_request_t cmd); +-int do_attr_command(cmd_request_t cmd); ++ ++/** ++ * @internal ++ * Carry out a geo-atribute related command ++ * ++ * @param[in,out] conf config object to refer to ++ * @param[in] cmd what to perform ++ * ++ * @return 0 or negative value (-1 or -errno) on error ++ */ ++int do_attr_command(struct booth_config *conf, cmd_request_t cmd); ++ + int process_attr_request(struct client *req_client, void *buf); + int attr_recv(void *buf, struct booth_site *source); + int store_geo_attr(struct ticket_config *tk, const char *name, const char *val, int notime); +diff --git a/src/config.c b/src/config.c +index 5e34919..5236505 100644 +--- a/src/config.c ++++ b/src/config.c +@@ -991,16 +991,18 @@ g_inval: + } + + +-static int get_other_site(struct booth_site **node) ++static int get_other_site(struct booth_config *conf, ++ struct booth_site **node) + { + struct booth_site *n; + int i; + + *node = NULL; +- if (!booth_conf) ++ if (conf == NULL) { + return 0; ++ } + +- _FOREACH_NODE(i, n) { ++ FOREACH_NODE(conf, i, n) { + if (n != local && n->type == SITE) { + if (!*node) { + *node = n; +@@ -1014,18 +1016,21 @@ static int get_other_site(struct booth_site **node) + } + + +-int find_site_by_name(char *site, struct booth_site **node, int any_type) ++int find_site_by_name(struct booth_config *conf, const char *site, ++ struct booth_site **node, int any_type) + { + struct booth_site *n; + int i; + +- if (!booth_conf) ++ if (conf == NULL) { + return 0; ++ } + +- if (!strcmp(site, OTHER_SITE)) +- return get_other_site(node); ++ if (!strcmp(site, OTHER_SITE)) { ++ return get_other_site(conf, node); ++ } + +- _FOREACH_NODE(i, n) { ++ FOREACH_NODE(conf, i, n) { + if ((n->type == SITE || any_type) && + strncmp(n->addr_string, site, sizeof(n->addr_string)) == 0) { + *node = n; +diff --git a/src/config.h b/src/config.h +index ad7aed2..1a7f40c 100644 +--- a/src/config.h ++++ b/src/config.h +@@ -355,7 +355,20 @@ int read_config(struct booth_config **conf, const char *path, int type); + */ + int check_config(struct booth_config *conf, int type); + +-int find_site_by_name(char *site, struct booth_site **node, int any_type); ++/** ++ * @internal ++ * Find site in booth configuration by resolved host name ++ * ++ * @param[in,out] conf config object to refer to ++ * @param[in] site name to match against previously resolved host names ++ * @param[out] node relevant tracked data when found ++ * @param[in] any_type whether or not to consider also non-site members ++ * ++ * @return 0 if nothing found, or 1 when found (node assigned accordingly) ++ */ ++int find_site_by_name(struct booth_config *conf, const char *site, ++ struct booth_site **node, int any_type); ++ + int find_site_by_id(uint32_t site_id, struct booth_site **node); + + const char *type_to_string(int type); +diff --git a/src/main.c b/src/main.c +index 48736b5..445655c 100644 +--- a/src/main.c ++++ b/src/main.c +@@ -394,7 +394,7 @@ static int setup_config(struct booth_config **conf, int type) + + /* Set "local" pointer, ignoring errors. */ + if (cl.type == DAEMON && cl.site[0]) { +- if (!find_site_by_name(cl.site, &local, 1)) { ++ if (!find_site_by_name(booth_conf, cl.site, &local, 1)) { + log_error("Cannot find \"%s\" in the configuration.", + cl.site); + return -EINVAL; +@@ -708,7 +708,7 @@ static int query_get_string_answer(cmd_request_t cmd) + + if (!*cl.site) + site = local; +- else if (!find_site_by_name(cl.site, &site, 1)) { ++ else if (!find_site_by_name(booth_conf, cl.site, &site, 1)) { + log_error("cannot find site \"%s\"", cl.site); + rv = ENOENT; + goto out; +@@ -782,7 +782,7 @@ static int do_command(cmd_request_t cmd) + if (!*cl.site) + site = local; + else { +- if (!find_site_by_name(cl.site, &site, 1)) { ++ if (!find_site_by_name(booth_conf, cl.site, &site, 1)) { + log_error("Site \"%s\" not configured.", cl.site); + goto out_close; + } +@@ -1617,7 +1617,7 @@ static int do_attr(struct booth_config **conf) + + case ATTR_SET: + case ATTR_DEL: +- rv = do_attr_command(cl.op); ++ rv = do_attr_command(booth_conf, cl.op); + break; + } + +-- +2.25.1 + diff --git a/backport-Refactor-Rename-FOREACH_-NODE-TICKET.patch b/backport-Refactor-Rename-FOREACH_-NODE-TICKET.patch new file mode 100644 index 0000000..1d71447 --- /dev/null +++ b/backport-Refactor-Rename-FOREACH_-NODE-TICKET.patch @@ -0,0 +1,240 @@ +From 6f515efdd0c3abc59711d29bed6d31032ef6d506 Mon Sep 17 00:00:00 2001 +From: Chris Lumens +Date: Thu, 11 Jul 2024 10:16:24 -0400 +Subject: [PATCH 1/5] Refactor: Rename FOREACH_{NODE,TICKET}. + +Future commits will continue to get rid of uses of the global config +variables. Some of those uses involve these macros, so we need a +version of them that takes a conf argument as well. + +This commit just temporarily renames those macros so not every caller +has to change. A later commit can introduce three argument versions of +these macros, and then we can move callers in several commits. When all +are moved, these renamed versions can go away. +--- + src/config.c | 6 +++--- + src/handler.c | 2 +- + src/main.c | 2 +- + src/raft.c | 2 +- + src/ticket.c | 22 +++++++++++----------- + src/ticket.h | 4 ++-- + src/transport.c | 4 ++-- + 7 files changed, 21 insertions(+), 21 deletions(-) + +diff --git a/src/config.c b/src/config.c +index 0dcffdc..5e34919 100644 +--- a/src/config.c ++++ b/src/config.c +@@ -1000,7 +1000,7 @@ static int get_other_site(struct booth_site **node) + if (!booth_conf) + return 0; + +- FOREACH_NODE(i, n) { ++ _FOREACH_NODE(i, n) { + if (n != local && n->type == SITE) { + if (!*node) { + *node = n; +@@ -1025,7 +1025,7 @@ int find_site_by_name(char *site, struct booth_site **node, int any_type) + if (!strcmp(site, OTHER_SITE)) + return get_other_site(node); + +- FOREACH_NODE(i, n) { ++ _FOREACH_NODE(i, n) { + if ((n->type == SITE || any_type) && + strncmp(n->addr_string, site, sizeof(n->addr_string)) == 0) { + *node = n; +@@ -1049,7 +1049,7 @@ int find_site_by_id(uint32_t site_id, struct booth_site **node) + if (!booth_conf) + return 0; + +- FOREACH_NODE(i, n) { ++ _FOREACH_NODE(i, n) { + if (n->site_id == site_id) { + *node = n; + return 1; +diff --git a/src/handler.c b/src/handler.c +index 9d74f0f..727b89c 100644 +--- a/src/handler.c ++++ b/src/handler.c +@@ -133,7 +133,7 @@ void wait_child(int sig) + /* use waitpid(2) and not wait(2) in order not to interfere + * with popen(2)/pclose(2) and system(2) used in pacemaker.c + */ +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + if (tk_test.path && tk_test.pid > 0 && + (tk_test.progstate == EXTPROG_RUNNING || + tk_test.progstate == EXTPROG_IGNORE) && +diff --git a/src/main.c b/src/main.c +index 3ad0290..48736b5 100644 +--- a/src/main.c ++++ b/src/main.c +@@ -233,7 +233,7 @@ static int format_peers(char **pdata, unsigned int *len) + return -ENOMEM; + + cp = data; +- FOREACH_NODE(i, s) { ++ _FOREACH_NODE(i, s) { + if (s == local) + continue; + strftime(time_str, sizeof(time_str), "%F %T", +diff --git a/src/raft.c b/src/raft.c +index dbe773c..1c73b51 100644 +--- a/src/raft.c ++++ b/src/raft.c +@@ -40,7 +40,7 @@ inline static void clear_election(struct ticket_config *tk) + + tk_log_debug("clear election"); + tk->votes_received = 0; +- FOREACH_NODE(i, site) { ++ _FOREACH_NODE(i, site) { + tk->votes_for[site->index] = NULL; + } + } +diff --git a/src/ticket.c b/src/ticket.c +index 1e2b50a..384a229 100644 +--- a/src/ticket.c ++++ b/src/ticket.c +@@ -65,7 +65,7 @@ int find_ticket_by_name(const char *ticket, struct ticket_config **found) + if (found) + *found = NULL; + +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + if (!strncmp(tk->name, ticket, sizeof(tk->name))) { + if (found) + *found = tk; +@@ -390,7 +390,7 @@ int list_ticket(char **pdata, unsigned int *len) + + alloc = booth_conf->ticket_count * (BOOTH_NAME_LEN * 2 + 128 + 16); + +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + multiple_grant_warning_length = number_sites_marked_as_granted(tk); + + if (multiple_grant_warning_length > 1) { +@@ -404,7 +404,7 @@ int list_ticket(char **pdata, unsigned int *len) + return -ENOMEM; + + cp = data; +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + if ((!is_manual(tk)) && is_time_set(&tk->term_expires)) { + /* Manual tickets doesn't have term_expires defined */ + ts = wall_ts(&tk->term_expires); +@@ -452,7 +452,7 @@ int list_ticket(char **pdata, unsigned int *len) + } + } + +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + multiple_grant_warning_length = number_sites_marked_as_granted(tk); + + if (multiple_grant_warning_length > 1) { +@@ -461,7 +461,7 @@ int list_ticket(char **pdata, unsigned int *len) + "\nWARNING: The ticket %s is granted to multiple sites: ", // ~55 characters + tk->name); + +- FOREACH_NODE(site_index, site) { ++ _FOREACH_NODE(site_index, site) { + if (tk->sites_where_granted[site_index] > 0) { + cp += snprintf(cp, + alloc - (cp - data), +@@ -617,7 +617,7 @@ int setup_ticket(void) + struct ticket_config *tk; + int i; + +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + reset_ticket(tk); + + if (local->type == SITE) { +@@ -844,7 +844,7 @@ static void log_lost_servers(struct ticket_config *tk) + */ + return; + +- FOREACH_NODE(i, n) { ++ _FOREACH_NODE(i, n) { + if (!(tk->acks_received & n->bitmask)) { + tk_log_warn("%s %s didn't acknowledge our %s, " + "will retry %d times", +@@ -864,7 +864,7 @@ static void resend_msg(struct ticket_config *tk) + if (!(tk->acks_received ^ local->bitmask)) { + ticket_broadcast(tk, tk->last_request, 0, RLT_SUCCESS, 0); + } else { +- FOREACH_NODE(i, n) { ++ _FOREACH_NODE(i, n) { + if (!(tk->acks_received & n->bitmask)) { + n->resend_cnt++; + tk_log_debug("resending %s to %s", +@@ -1128,7 +1128,7 @@ void process_tickets(void) + int i; + timetype last_cron; + +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + if (!has_extprog_exited(tk) && + is_time_set(&tk->next_cron) && !is_past(&tk->next_cron)) + continue; +@@ -1152,7 +1152,7 @@ void tickets_log_info(void) + int i; + time_t ts; + +- FOREACH_TICKET(i, tk) { ++ _FOREACH_TICKET(i, tk) { + ts = wall_ts(&tk->term_expires); + tk_log_info("state '%s' " + "term %d " +@@ -1350,7 +1350,7 @@ int number_sites_marked_as_granted(struct ticket_config *tk) + int i, result = 0; + struct booth_site *ignored __attribute__((unused)); + +- FOREACH_NODE(i, ignored) { ++ _FOREACH_NODE(i, ignored) { + result += tk->sites_where_granted[i]; + } + +diff --git a/src/ticket.h b/src/ticket.h +index 805bcef..af4b195 100644 +--- a/src/ticket.h ++++ b/src/ticket.h +@@ -35,12 +35,12 @@ extern int TIME_RES; + #define DEFAULT_RETRIES 10 + + +-#define FOREACH_TICKET(i_, t_) \ ++#define _FOREACH_TICKET(i_, t_) \ + for (i_ = 0; \ + (t_ = booth_conf->ticket + i_, i_ < booth_conf->ticket_count); \ + i_++) + +-#define FOREACH_NODE(i_, n_) \ ++#define _FOREACH_NODE(i_, n_) \ + for (i_ = 0; \ + (n_ = booth_conf->site + i_, i_ < booth_conf->site_count); \ + i_++) +diff --git a/src/transport.c b/src/transport.c +index afd6512..4fb754e 100644 +--- a/src/transport.c ++++ b/src/transport.c +@@ -98,7 +98,7 @@ static int find_address(unsigned char ipaddr[BOOTH_IPADDR_LEN], + /* One bit left to check means ignore 7 lowest bits. */ + mask = ~( (1 << (8 - bits_left)) -1); + +- FOREACH_NODE(i, node) { ++ _FOREACH_NODE(i, node) { + if (family != node->family) + continue; + n_a = node_to_addr_pointer(node); +@@ -900,7 +900,7 @@ static int booth_udp_broadcast_auth(void *buf, int len) + return rv; + + rvs = 0; +- FOREACH_NODE(i, site) { ++ _FOREACH_NODE(i, site) { + if (site != local) { + rv = booth_udp_send(site, buf, len); + if (!rvs) +-- +2.25.1 + diff --git a/booth.spec b/booth.spec index 92f923a..2777df1 100644 --- a/booth.spec +++ b/booth.spec @@ -24,7 +24,7 @@ %bcond_with run_build_tests %bcond_with include_unit_test -%global release 4 +%global release 5 ## User and group to use for nonprivileged services (should be in sync with pacemaker) %global uname hacluster @@ -58,6 +58,11 @@ patch07: backport-build-Add-support-for-building-with-mock.patch patch08: backport-Refactor-Remove-the-unused-check_site-function.patch patch09: backport-Refactor-Remove-global-booth_conf-variable-in-read_c.patch patch10: backport-Refactor-Remove-global-booth_conf-variable-in-check_.patch +patch11: backport-Refactor-Rename-FOREACH_-NODE-TICKET.patch +patch12: backport-Refactor-Add-three-argument-versions-of-FOREACH_-NOD.patch +patch13: backport-Refactor-Remove-global-booth_conf-variable-in-find_name.patch +patch14: backport-Refactor-Remove-global-booth_conf-variable-in-find_id.patch +patch15: backport-Refactor-Remove-global-booth_conf-variable-in-find_myself.patch # direct build process dependencies BuildRequires: autoconf @@ -306,6 +311,9 @@ VERBOSE=1 make check %{_usr}/lib/ocf/resource.d/booth/sharedrsc %changelog +* Thu Oct 17 2024 bizhiyuan -1.2-5 +- Apply find_* function refactorings + * Thu Sep 19 2024 bizhiyuan -1.2-4 - Apply config.c refactorings -- Gitee