Maintenance refresh, part 3: code cleanup

src/attr.c

@@ -17,7 +17,9 @@
  */
 
 #include <stdio.h>
+#include <string.h>
 #include "attr.h"
+#include "booth.h"
 #include "ticket.h"
 #include "pacemaker.h"
 
@@ -244,7 +246,8 @@ static void free_geo_attr(gpointer data)
 	g_free(a);
 }
 
-int store_geo_attr(struct ticket_config *tk, const char *name, char *val, int notime)
+int store_geo_attr(struct ticket_config *tk, const char *name,
+		   const char *val, int notime)
 {
 	struct geo_attr *a;
 	GDestroyNotify free_geo_attr_notify = free_geo_attr;
@@ -264,18 +267,26 @@ int store_geo_attr(struct ticket_config *tk, const char *name, char *val, int no
 		return -1;
 	}
 
-	a = (struct geo_attr *)calloc(1, sizeof(struct geo_attr));
-	if (!a) {
-		log_error("out of memory");
-		return -1;
-	}
+	if (strnlen(name, BOOTH_NAME_LEN) == BOOTH_NAME_LEN)
+		log_warn("name of the attribute too long (%d+ bytes), skipped",
+			 BOOTH_NAME_LEN);
+	else if (strnlen(val, BOOTH_ATTRVAL_LEN) == BOOTH_ATTRVAL_LEN)
+		log_warn("value of the attribute too long (%d+ bytes), skipped",
+			 BOOTH_ATTRVAL_LEN);
+	else {
+		a = (struct geo_attr *)calloc(1, sizeof(struct geo_attr));
+		if (!a) {
+			log_error("out of memory");
+			return -1;
+		}
 
-	a->val = g_strdup(val);
-	if (!notime)
-		get_time(&a->update_ts);
+		a->val = g_strdup(val);
+		if (!notime)
+			get_time(&a->update_ts);
 
-	g_hash_table_insert(tk->attr,
-		g_strndup(name, BOOTH_NAME_LEN), a);
+		g_hash_table_insert(tk->attr,
+			g_strdup(name), a);
+	}
 
 	return 0;
 }

src/attr.h

@@ -34,6 +34,6 @@ int test_attr_reply(cmd_result_t reply_code, cmd_request_t cmd);
 int do_attr_command(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, char *val, int notime);
+int store_geo_attr(struct ticket_config *tk, const char *name, const char *val, int notime);
 
 #endif /* _ATTR_H */

src/auth.c

@@ -24,7 +24,7 @@
  * at result for the MAC
  */
 int calc_hmac(const void *data, size_t datalen,
-	int hid, unsigned char *result, char *key, int keylen)
+	int hid, unsigned char *result, char *key, unsigned int keylen)
 {
 	static gcry_md_hd_t digest;
 	gcry_error_t err;

src/auth.h

@@ -27,7 +27,7 @@
 #define BOOTH_HASH GCRY_MD_SHA1
 
 int calc_hmac(const void *data, size_t datalen,
-	int hid, unsigned char *result, char *key, int keylen);
+	int hid, unsigned char *result, char *key, unsigned int keylen);
 int verify_hmac(const void *data, size_t datalen,
 	int hid, unsigned char *hmac, char *key, int keylen);
 #endif

src/booth.h

@@ -77,10 +77,10 @@
 #define TICKET_LOST CHAR2CONST('L', 'O', 'S', 'T')
 
 
-typedef unsigned char boothc_site  [BOOTH_NAME_LEN];
-typedef unsigned char boothc_ticket[BOOTH_NAME_LEN];
-typedef unsigned char boothc_attr[BOOTH_NAME_LEN];
-typedef unsigned char boothc_attr_value[BOOTH_ATTRVAL_LEN];
+typedef char boothc_site[BOOTH_NAME_LEN];
+typedef char boothc_ticket[BOOTH_NAME_LEN];
+typedef char boothc_attr[BOOTH_NAME_LEN];
+typedef char boothc_attr_value[BOOTH_ATTRVAL_LEN];
 
 /* message option bits */
 enum {
@@ -290,7 +290,7 @@ struct booth_site {
 	/** Roles, like ACCEPTOR, PROPOSER, or LEARNER. Not really used ATM. */
 	int role;
 
-	char addr_string[BOOTH_NAME_LEN];
+	boothc_site addr_string;
 
 	int tcp_fd;
 	int udp_fd;

src/config.c

@@ -61,8 +61,7 @@ static int ticket_realloc(void)
 }
 
 
-int add_site(char *address, int type);
-int add_site(char *addr_string, int type)
+static int add_site(char *addr_string, int type)
 {
 	int rv;
 	struct booth_site *site;
@@ -76,7 +75,8 @@ int add_site(char *addr_string, int type)
 		log_error("too many nodes");
 		goto out;
 	}
-	if (strlen(addr_string)+1 >= sizeof(booth_conf->site[0].addr_string)) {
+	if (strnlen(addr_string, sizeof(booth_conf->site[0].addr_string))
+			>= sizeof(booth_conf->site[0].addr_string)) {
 		log_error("site address \"%s\" too long", addr_string);
 		goto out;
 	}
@@ -87,7 +87,7 @@ int add_site(char *addr_string, int type)
 	site->type = type;
 	/* Make site_id start at a non-zero point.
 	 * Perhaps use hash over string or address? */
-	strcpy(site->addr_string, addr_string);
+	strncpy(site->addr_string, addr_string, sizeof(site->addr_string));
 
 
 	site->index = booth_conf->site_count;
@@ -909,7 +909,7 @@ static int get_other_site(struct booth_site **node)
 }
 
 
-int find_site_by_name(unsigned char *site, struct booth_site **node, int any_type)
+int find_site_by_name(char *site, struct booth_site **node, int any_type)
 {
 	struct booth_site *n;
 	int i;
@@ -923,7 +923,7 @@ int find_site_by_name(unsigned char *site, struct booth_site **node, int any_typ
 	for (i = 0; i < booth_conf->site_count; i++) {
 		n = booth_conf->site + i;
 		if ((n->type == SITE || any_type) &&
-		    strcmp(n->addr_string, site) == 0) {
+		    strncmp(n->addr_string, site, sizeof(n->addr_string)) == 0) {
 			*node = n;
 			return 1;
 		}

src/config.h

@@ -260,7 +260,7 @@ struct booth_config {
     /** File containing the authentication file. */
 	char authfile[BOOTH_PATH_LEN];
 	struct stat authstat;
-	unsigned char authkey[BOOTH_MAX_KEY_LEN];
+	char authkey[BOOTH_MAX_KEY_LEN];
 	int authkey_len;
     /** Maximum time skew between peers allowed */
 	int maxtimeskew;
@@ -297,7 +297,7 @@ int read_config(const char *path, int type);
 
 int check_config(int type);
 
-int find_site_by_name(unsigned char *site, struct booth_site **node, int any_type);
+int find_site_by_name(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);

src/inline-fn.h

@@ -146,24 +146,6 @@ static inline void init_ticket_msg(struct boothc_ticket_msg *msg,
 	}
 }
 
-static inline void init_attr_msg(struct boothc_attr_msg *msg,
-		int cmd, int request, int rv, int reason,
-		struct ticket_config *tk, char *attr_name, struct geo_attr *attr)
-{
-	assert(tk);
-	assert(attr);
-
-	init_header(&msg->header, cmd, request, 0, rv, reason, sizeof(*msg));
-
-	if (!tk) {
-		memset(&msg->attr.tkt_id, 0, sizeof(msg->attr.tkt_id));
-	} else {
-		memcpy(msg->attr.tkt_id, tk->name, sizeof(msg->attr.tkt_id));
-	}
-	memcpy(msg->attr.name, attr_name, sizeof(msg->attr.name));
-	memcpy(msg->attr.val, attr->val, sizeof(msg->attr.val));
-}
-
 
 static inline struct booth_transport const *transport(void)
 {

src/main.c

@@ -256,7 +256,7 @@ static int format_peers(char **pdata, unsigned int *len)
 void list_peers(int fd)
 {
 	char *data;
-	int olen;
+	unsigned int olen;
 	struct boothc_hdr_msg hdr;
 
 	if (format_peers(&data, &olen) < 0)
@@ -274,7 +274,7 @@ void list_peers(int fd)
  */
 static void trim_key()
 {
-	unsigned char *p;
+	char *p;
 	int i;
 
 	for (i=0, p=booth_conf->authkey; i < booth_conf->authkey_len; i++, p++)
@@ -746,7 +746,8 @@ static int do_command(cmd_request_t cmd)
 	if (!cl.msg.ticket.id[0]) {
 		/* If the loaded configuration has only a single ticket defined, use that. */
 		if (booth_conf->ticket_count == 1) {
-			strcpy(cl.msg.ticket.id, booth_conf->ticket[0].name);
+			strncpy(cl.msg.ticket.id, booth_conf->ticket[0].name,
+				sizeof(cl.msg.ticket.id));
 		} else {
 			log_error("No ticket given.");
 			goto out_close;
@@ -1457,7 +1458,7 @@ static int do_server(int type)
 		return rv;
 
 	if (cl_enable_coredumps(TRUE) < 0){
-		cl_log(LOG_ERR, "enabling core dump failed");
+		log_error("enabling core dump failed");
 	}
 	cl_cdtocoredir();
 	prctl(PR_SET_DUMPABLE, (unsigned long)TRUE, 0UL, 0UL, 0UL);
@@ -1510,7 +1511,8 @@ static int do_attr(void)
 	if (!cl.attr_msg.attr.tkt_id[0]) {
 		/* If the loaded configuration has only a single ticket defined, use that. */
 		if (booth_conf->ticket_count == 1) {
-			strcpy(cl.attr_msg.attr.tkt_id, booth_conf->ticket[0].name);
+			strncpy(cl.attr_msg.attr.tkt_id, booth_conf->ticket[0].name,
+				sizeof(cl.attr_msg.attr.tkt_id));
 		} else {
 			rv = 1;
 			log_error("No ticket given.");
@@ -1537,7 +1539,7 @@ static int do_attr(void)
 int main(int argc, char *argv[], char *envp[])
 {
 	int rv;
-	char *cp;
+	const char *cp;
 
 	init_set_proc_title(argc, argv, envp);
 	get_time(&start_time);

src/pacemaker.c

@@ -265,27 +265,30 @@ static int pcmk_store_ticket_nonatomic(struct ticket_config *tk)
 	return rv;
 }
 
-typedef int (*attr_f)(struct ticket_config *tk, const char *name, char *val);
+typedef int (*attr_f)(struct ticket_config *tk, const char *name,
+		      const char *val);
 
 struct attr_tab
 {
 	const char *name;
 	attr_f handling_f;
 };
 
-static int save_expires(struct ticket_config *tk, const char *name, char *val)
+static int save_expires(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, char *val)
+static int save_term(struct ticket_config *tk, const char *name,
+		     const char *val)
 {
 	tk->current_term = atol(val);
 	return 0;
 }
 
-static int parse_boolean(char *val)
+static int parse_boolean(const char *val)
 {
 	long v;
 
@@ -299,25 +302,29 @@ static int parse_boolean(char *val)
 	return v;
 }
 
-static int save_granted(struct ticket_config *tk, const char *name, char *val)
+static int save_granted(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, char *val)
+static int save_owner(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);
 }
 
-static int ignore_attr(struct ticket_config *tk, const char *name, char *val)
+static int ignore_attr(struct ticket_config *tk, const char *name,
+		       const char *val)
 {
 	return 0;
 }
 
-static int save_attr(struct ticket_config *tk, const char *name, char *val)
+static int save_attr(struct ticket_config *tk, const char *name,
+		     const char *val)
 {
 	/* tell store_geo_attr not to store time, we don't have that
 	 * information available
@@ -397,13 +404,15 @@ static int save_attributes(struct ticket_config *tk, xmlDocPtr doc)
 	for (attr = n->properties; attr; attr = attr->next) {
 		v = xmlGetProp(n, attr->name);
 		for (atp = attr_handlers; atp->name; atp++) {
-			if (!strcmp(atp->name, attr->name)) {
-				rc = atp->handling_f(tk, attr->name, v);
+			if (!strcmp(atp->name, (const char *) attr->name)) {
+				rc = atp->handling_f(tk, (const char *) attr->name,
+						     (const char *) v);
 				break;
 			}
 		}
 		if (!atp->name) {
-			rc = save_attr(tk, attr->name, v);
+			rc = save_attr(tk, (const char *) attr->name,
+				       (const char *) v);
 		}
 		if (rc) {
 			tk_log_error("error storing attribute %s", attr->name);
@@ -446,7 +455,7 @@ static int parse_ticket_state(struct ticket_config *tk, FILE *p)
 		}
 	}
 
-	doc = xmlReadDoc(input->str, NULL, NULL, opts);
+	doc = xmlReadDoc((const xmlChar *) input->str, NULL, NULL, opts);
 	if (doc == NULL) {
 		errptr = xmlGetLastError();
 		if (errptr) {

src/ticket.c

@@ -59,7 +59,8 @@ int find_ticket_by_name(const char *ticket, struct ticket_config **found)
 		*found = NULL;
 
 	for (i = 0; i < booth_conf->ticket_count; i++) {
-		if (!strcmp(booth_conf->ticket[i].name, ticket)) {
+		if (!strncmp(booth_conf->ticket[i].name, ticket,
+			     sizeof(booth_conf->ticket[i].name))) {
 			if (found)
 				*found = booth_conf->ticket + i;
 			return 1;
@@ -564,7 +565,8 @@ int setup_ticket(void)
 int ticket_answer_list(int fd)
 {
 	char *data;
-	int olen, rv;
+	int rv;
+	unsigned int olen;
 	struct boothc_hdr_msg hdr;
 
 	rv = list_ticket(&data, &olen);