Skip to content

Commit

Permalink
Refactor: call find_site_by_name just once, up the stream
Browse files Browse the repository at this point in the history 
Respective logic was duplicated for all "booth list/peers/grant/revoke"
and "geostore list/get/set/del" separately, so utilize a natural control
flow to carry this once-resolved target site from here, sharing it with
the special case of "daemon" role invoked with "-s site" (debug mode).
Side effect: simpler, terser code.
  • Loading branch information
@jnpkrn
jnpkrn committed Sep 15, 2016
1 parent 1185487 commit 0c94ea1
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 56 deletions.
16 changes: 2 additions & 14 deletions src/attr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,24 +150,13 @@ static int read_server_reply(
return rv;
}

int do_attr_command(cmd_request_t cmd)
int do_attr_command(cmd_request_t cmd, struct booth_site *site)
{
struct booth_site *site = NULL;
struct boothc_header *header;
struct booth_transport const *tpt;
int len, rv = -1;
char *msg = NULL;

if (!*cl.site)
site = local;
else {
if (!find_site_by_name(cl.site, &site, 1)) {
log_error("Site \"%s\" not configured.", cl.site);
rv = -ENOENT;
goto out_close;
}
}

if (site->type == ARBITRATOR) {
if (site == local) {
log_error("We're just an arbitrator, no attributes here.");
Expand Down Expand Up @@ -220,8 +209,7 @@ int do_attr_command(cmd_request_t cmd)
rv = test_attr_reply(ntohl(header->result), cmd);

out_close:
if (site)
tpt->close(site);
tpt->close(site);
if (msg)
free(msg);
return rv;
Expand Down
2 changes: 1 addition & 1 deletion src/attr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

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);
int do_attr_command(cmd_request_t cmd, struct booth_site *site);
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);
Expand Down
75 changes: 34 additions & 41 deletions src/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ int update_authkey()
return 0;
}

static int setup_config(int type)
static int setup_config(int type, struct booth_site **site)
{
int rv;

Expand All @@ -369,18 +369,31 @@ static int setup_config(int type)
#endif
}

/* Set "local" pointer, ignoring errors. */
if (cl.type == DAEMON && cl.site[0]) {
if (!find_site_by_name(cl.site, &local, 1)) {
log_error("Cannot find \"%s\" (myself) in the configuration.",
cl.site);
return -EINVAL;
/* Determine the target based on the provided address, ignore
errors with DAEMON (special debug/testing arrangement). */
if (*cl.site && (cl.type == DAEMON || (site && strcmp(cl.site, OTHER_SITE)))) {
if (!find_site_by_name(cl.site, cl.type == DAEMON ? &local : site, 1)) {
log_error("Cannot find \"%s\"%s in the configuration.",
cl.site, cl.type == DAEMON ? " (myself)" : "");
if (cl.type != DAEMON)
return -EINVAL;
}
local->local = 1;
} else if (!find_myself(NULL, type == CLIENT || type == GEOSTORE)) {
if (cl.type == DAEMON)
local->local = 1;
else
site = NULL; /* prevent from overwriting */
}
/* Self-determine us. */
if (!find_myself(site, type == CLIENT || type == GEOSTORE)) {
log_error("Cannot find myself in the configuration.");
return -EINVAL;
}
/* We can resolve "other" only after we've determined us. */
if (*cl.site && site && !strcmp(cl.site, OTHER_SITE)
&& !find_site_by_name(cl.site, site, 1)) {
log_error("Cannot find %s node in the configuration.", cl.site);
return -EINVAL;
}

rv = check_config(type);
if (rv < 0)
Expand Down Expand Up @@ -635,9 +648,8 @@ static int test_reply(cmd_result_t reply_code, cmd_request_t cmd)
return rv;
}

static int query_get_string_answer(cmd_request_t cmd)
static int query_get_string_answer(cmd_request_t cmd, struct booth_site *site)
{
struct booth_site *site;
struct boothc_hdr_msg reply;
struct boothc_header *header;
char *data;
Expand All @@ -662,14 +674,6 @@ static int query_get_string_answer(cmd_request_t cmd)

init_header(header, cmd, 0, cl.options, 0, 0, msg_size);

if (!*cl.site)
site = local;
else if (!find_site_by_name(cl.site, &site, 1)) {
log_error("cannot find site \"%s\"", cl.site);
rv = -ENOENT;
goto out;
}

tpt = booth_transport + TCP;
rv = tpt->open(site);
if (rv < 0)
Expand Down Expand Up @@ -709,16 +713,14 @@ static int query_get_string_answer(cmd_request_t cmd)
rv = test_reply_f(ntohl(reply.header.result), cmd);
out_close:
tpt->close(site);
out:
if (data)
free(data);
return rv;
}


static int do_command(cmd_request_t cmd)
static int do_command(cmd_request_t cmd, struct booth_site *site)
{
struct booth_site *site;
struct boothc_ticket_msg reply;
struct booth_transport const *tpt;
uint32_t leader_id;
Expand All @@ -732,21 +734,10 @@ static int do_command(cmd_request_t cmd)
op_str = "revoke";

rv = 0;
site = NULL;

/* Always use TCP for client - at least for now. */
tpt = booth_transport + TCP;

if (!*cl.site)
site = local;
else {
if (!find_site_by_name(cl.site, &site, 1)) {
log_error("Site \"%s\" not configured.", cl.site);
rv = -ENOENT;
goto out_close;
}
}

if (site->type == ARBITRATOR) {
if (site == local) {
log_error("We're just an arbitrator, cannot grant/revoke tickets here.");
Expand Down Expand Up @@ -1296,7 +1287,7 @@ static int do_status(int type)

ret = PCMK_OCF_NOT_RUNNING;

rv = setup_config(type);
rv = setup_config(type, NULL);
if (rv) {
reason = "Error reading configuration.";
ret = PCMK_OCF_UNKNOWN_ERROR;
Expand Down Expand Up @@ -1412,7 +1403,7 @@ static int do_server(int type)
int rv = -1;
static char log_ent[128] = DAEMON_NAME "-";

rv = setup_config(type);
rv = setup_config(type, NULL);
if (rv < 0)
return rv;

Expand Down Expand Up @@ -1478,8 +1469,9 @@ static int do_server(int type)
static int do_client(void)
{
int rv;
struct booth_site *site;

rv = setup_config(CLIENT);
rv = setup_config(CLIENT, &site);
if (rv < 0) {
log_error("cannot read config");
goto out;
Expand All @@ -1488,12 +1480,12 @@ static int do_client(void)
switch (cl.op) {
case CMD_LIST:
case CMD_PEERS:
rv = query_get_string_answer(cl.op);
rv = query_get_string_answer(cl.op, site);
break;

case CMD_GRANT:
case CMD_REVOKE:
rv = do_command(cl.op);
rv = do_command(cl.op, site);
break;
}

Expand All @@ -1504,8 +1496,9 @@ static int do_client(void)
static int do_attr(void)
{
int rv = -1;
struct booth_site *site;

rv = setup_config(GEOSTORE);
rv = setup_config(GEOSTORE, &site);
if (rv < 0) {
log_error("cannot read config");
goto out;
Expand All @@ -1529,12 +1522,12 @@ static int do_attr(void)
switch (cl.op) {
case ATTR_LIST:
case ATTR_GET:
rv = query_get_string_answer(cl.op);
rv = query_get_string_answer(cl.op, site);
break;

case ATTR_SET:
case ATTR_DEL:
rv = do_attr_command(cl.op);
rv = do_attr_command(cl.op, site);
break;
}

Expand Down

0 comments on commit 0c94ea1

Please sign in to comment.