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.

Sign up for GitHub

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

Jump to bottom

Fix a segfault + related refactoring #50

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix a segfault + related refactoring #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b8a336
Low: make find_site_by_name failure set error code
jnpkrn Sep 13, 2016
bcf1117
High: ensure local site resolved for all effective actions
jnpkrn Sep 13, 2016
1185487
Low: make daemon with "-s site" (debug mode) claim "myself"
jnpkrn Sep 13, 2016
0c94ea1
Refactor: call find_site_by_name just once, up the stream
jnpkrn Sep 13, 2016
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
16 changes: 3 additions & 13 deletions src/attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

#include <errno.h>
#include <stdio.h>
#include <string.h>
#include "attr.h"
Expand Down Expand Up @@ -149,23 +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);
goto out_close;
}
}

if (site->type == ARBITRATOR) {
if (site == local) {
log_error("We're just an arbitrator, no attributes here.");
Expand Down Expand Up @@ -218,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
Open in desktop
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
91 changes: 37 additions & 54 deletions src/main.c
View file
Open in desktop
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,17 +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\" 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
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 @@ -634,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 @@ -661,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 @@ -708,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 @@ -731,20 +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);
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 @@ -1294,20 +1287,13 @@ 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;
goto quit;
}


if (!local) {
reason = "No Service IP active here.";
goto quit;
}


rv = _lockfile(O_RDWR, &status_lock_fd, &pid);
if (status_lock_fd == -1) {
reason = "No PID file.";
Expand Down Expand Up @@ -1417,15 +1403,10 @@ 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;

if (!local) {
log_error("Cannot find myself in the configuration.");
exit(EXIT_FAILURE);
}

if (daemonize) {
if (daemon(0, 0) < 0) {
perror("daemon error");
Expand Down Expand Up @@ -1488,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 @@ -1498,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 @@ -1514,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 @@ -1539,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