From 9e869ebe3a5127575105d82c4d289d95cbed2db9 Mon Sep 17 00:00:00 2001
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Mon, 25 Mar 2019 17:02:01 +1100
Subject: Various fixups and checks to make scan-build happy

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
 discover/device-handler.c     |  2 +-
 discover/discover-server.c    | 15 +++++++++------
 discover/event.c              |  1 -
 discover/grub2/env.c          |  2 ++
 discover/grub2/script.c       |  5 ++++-
 discover/ipmi.c               |  1 -
 discover/paths.c              |  3 ++-
 discover/pxe-parser.c         |  7 ++++++-
 discover/user-event.c         | 19 +++++++++++++------
 discover/yaboot-parser.c      |  4 ++++
 lib/pb-config/pb-config.c     | 12 +++++++++---
 lib/pb-protocol/pb-protocol.c | 39 +++++++++++++++++----------------------
 ui/ncurses/nc-cui.c           |  4 +++-
 utils/hooks/30-dtb-updates.c  |  2 +-
 14 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/discover/device-handler.c b/discover/device-handler.c
index d41bb4b..d85f1af 100644
--- a/discover/device-handler.c
+++ b/discover/device-handler.c
@@ -1209,7 +1209,7 @@ void device_handler_add_ramdisk(struct device_handler *handler,
 	}
 
 	handler->ramdisks[i] = dev;
-	i = handler->n_ramdisks++;
+	handler->n_ramdisks++;
 }
 
 struct ramdisk_device *device_handler_get_ramdisk(
diff --git a/discover/discover-server.c b/discover/discover-server.c
index 1a332cb..e29ce27 100644
--- a/discover/discover-server.c
+++ b/discover/discover-server.c
@@ -298,7 +298,7 @@ static int discover_server_handle_auth_message(struct client *client,
 {
 	struct status *status;
 	char *hash;
-	int rc;
+	int rc = 0;
 
 	status = talloc_zero(client, struct status);
 
@@ -403,7 +403,7 @@ static int discover_server_process_message(void *arg)
 	struct client *client = arg;
 	struct config *config;
 	char *url;
-	int rc;
+	int rc = 0;
 
 	message = pb_protocol_read_message(client, client->fd);
 
@@ -460,7 +460,7 @@ static int discover_server_process_message(void *arg)
 				talloc_free(status);
 			}
 		}
-		return 0;
+		return rc;
 	}
 
 	switch (message->action) {
@@ -537,7 +537,7 @@ static int discover_server_process_message(void *arg)
 			break;
 		}
 
-		rc = discover_server_handle_auth_message(client, auth_msg);
+		discover_server_handle_auth_message(client, auth_msg);
 		talloc_free(auth_msg);
 		break;
 	default:
@@ -791,8 +791,11 @@ struct discover_server *discover_server_init(struct waitset *waitset)
 	/* Allow all clients to communicate on this socket */
 	group = getgrnam("petitgroup");
 	if (group) {
-		chown(PB_SOCKET_PATH, 0, group->gr_gid);
-		chmod(PB_SOCKET_PATH, 0660);
+		if (chown(PB_SOCKET_PATH, 0, group->gr_gid))
+			pb_log_fn("Error setting socket ownership: %m\n");
+		errno = 0;
+		if (chmod(PB_SOCKET_PATH, 0660))
+			pb_log_fn("Error setting socket permissions: %m\n");
 	}
 
 	if (listen(server->socket, 8)) {
diff --git a/discover/event.c b/discover/event.c
index ec5537a..4c46d41 100644
--- a/discover/event.c
+++ b/discover/event.c
@@ -101,7 +101,6 @@ static void event_parse_params(struct event *event, const char *buf, int len)
 		sep = memchr(buf, '=', param_len);
 		if (!sep) {
 			name_len = param_len;
-			value_len = 0;
 			param->value = "";
 		} else {
 			name_len = sep - buf;
diff --git a/discover/grub2/env.c b/discover/grub2/env.c
index 7eda095..74d5729 100644
--- a/discover/grub2/env.c
+++ b/discover/grub2/env.c
@@ -86,6 +86,8 @@ int builtin_load_env(struct grub2_script *script,
 
 	if (!rc) {
 		rc = parse_buf_to_env(script, buf, len);
+		if (rc)
+			pb_debug_fn("Failed to set env\n");
 		talloc_free(buf);
 	}
 
diff --git a/discover/grub2/script.c b/discover/grub2/script.c
index 1a802b9..902df90 100644
--- a/discover/grub2/script.c
+++ b/discover/grub2/script.c
@@ -227,7 +227,7 @@ static void process_expansions(struct grub2_script *script,
 	}
 
 	/* we may have allocated an extra argv element but not populated it */
-	if (!argv->argv[argv->argc - 1])
+	if (argv->argv && !argv->argv[argv->argc - 1])
 		argv->argc--;
 }
 
@@ -489,6 +489,9 @@ void script_execute(struct grub2_script *script)
 {
 	struct discover_boot_option *opt, *tmp;
 
+	if (!script)
+		return;
+
 	init_env(script);
 	statements_execute(script, script->statements);
 
diff --git a/discover/ipmi.c b/discover/ipmi.c
index ae02bb0..66b465e 100644
--- a/discover/ipmi.c
+++ b/discover/ipmi.c
@@ -306,7 +306,6 @@ int parse_ipmi_interface_override(struct config *config, uint8_t *buf,
 			return -1;
 		}
 		ifconf->static_config.gateway = gatewaystr;
-		i += ipsize;
 	}
 
 	ifconf->override = true;
diff --git a/discover/paths.c b/discover/paths.c
index 54b843e..16fdd59 100644
--- a/discover/paths.c
+++ b/discover/paths.c
@@ -450,7 +450,8 @@ static void load_local(struct load_task *task)
 		result->status = LOAD_OK;
 	}
 
-	task->async_cb(task->result, task->async_data);
+	if (task->async_cb)
+		task->async_cb(task->result, task->async_data);
 }
 
 static void load_url_async_start_pending(struct load_task *task, int flags)
diff --git a/discover/pxe-parser.c b/discover/pxe-parser.c
index ba0f81c..035794c 100644
--- a/discover/pxe-parser.c
+++ b/discover/pxe-parser.c
@@ -292,9 +292,14 @@ static bool ipxe_simple_parser(struct conf_context *ctx, char *buf, int len)
 			continue;
 		}
 
+		if (!name) {
+			pb_debug_fn("missing name from conf_get_pair\n");
+			continue;
+		}
+
 		/* All other parameters require a value */
 		if (!value) {
-			pb_debug("%s: '%s' missing value\n", __func__, name);
+			pb_debug_fn("'%s' missing value\n", name);
 			continue;
 		}
 
diff --git a/discover/user-event.c b/discover/user-event.c
index d3d4a5e..cc03ffd 100644
--- a/discover/user-event.c
+++ b/discover/user-event.c
@@ -657,10 +657,10 @@ static void user_event_handle_message(struct user_event *uev, char *buf,
 		break;
 	case EVENT_ACTION_URL:
 		result = user_event_url(uev, event);
-		goto out;
+		break;
 	case EVENT_ACTION_DHCP:
 		result = user_event_dhcp(uev, event);
-		goto out;
+		break;
 	case EVENT_ACTION_BOOT:
 		result = user_event_boot(uev, event);
 		break;
@@ -671,13 +671,17 @@ static void user_event_handle_message(struct user_event *uev, char *buf,
 		result = user_event_plugin(uev, event);
 		break;
 	default:
+		result = -1;
 		break;
 	}
 
+	if (result)
+		pb_log_fn("failed to handle action %d\n", event->action);
+
 	/* user_event_url() and user_event_dhcp() will steal the event context,
 	 * but all others still need to free */
-	talloc_free(event);
-out:
+	if (talloc_parent(event) == uev)
+		talloc_free(event);
 	return;
 }
 
@@ -751,8 +755,11 @@ struct user_event *user_event_init(struct device_handler *handler,
 	}
 
 	/* Don't allow events from non-priviledged users */
-	chown(PBOOT_USER_EVENT_SOCKET, 0, 0);
-	chmod(PBOOT_USER_EVENT_SOCKET, 0660);
+	if (chown(PBOOT_USER_EVENT_SOCKET, 0, 0))
+		pb_log_fn("Error setting socket ownership: %m\n");
+	errno = 0;
+	if (chmod(PBOOT_USER_EVENT_SOCKET, 0660))
+		pb_log_fn("Error setting socket permissions: %m\n");
 
 	waiter_register_io(waitset, uev->socket, WAIT_IN,
 			user_event_process, uev);
diff --git a/discover/yaboot-parser.c b/discover/yaboot-parser.c
index b06248f..d0a40b1 100644
--- a/discover/yaboot-parser.c
+++ b/discover/yaboot-parser.c
@@ -213,6 +213,8 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name,
 
 		/* Then start the new image. */
 		opt = state_start_new_option(conf, state);
+		if (!opt)
+			pb_debug_fn("new opt is NULL\n");
 
 		state->boot_image = talloc_strdup(state, value);
 
@@ -235,6 +237,8 @@ static void yaboot_process_pair(struct conf_context *conf, const char *name,
 
 		/* Then start the new image. */
 		opt = state_start_new_option(conf, state);
+		if (!opt)
+			pb_debug_fn("new opt is NULL\n");
 
 		if (*value == '/') {
 			state->boot_image = talloc_strdup(state, value);
diff --git a/lib/pb-config/pb-config.c b/lib/pb-config/pb-config.c
index a802c92..735cd98 100644
--- a/lib/pb-config/pb-config.c
+++ b/lib/pb-config/pb-config.c
@@ -43,6 +43,9 @@ struct config *config_copy(void *ctx, const struct config *src)
 	struct config *dest;
 	unsigned int i;
 
+	if (!src)
+		return NULL;
+
 	dest = talloc_zero(ctx, struct config);
 	dest->autoboot_enabled = src->autoboot_enabled;
 	dest->autoboot_timeout_sec = src->autoboot_timeout_sec;
@@ -88,11 +91,14 @@ struct config *config_copy(void *ctx, const struct config *src)
 	dest->allow_writes = src->allow_writes;
 
 	dest->n_consoles = src->n_consoles;
-	if (src->consoles)
+	if (src->consoles) {
 		dest->consoles = talloc_array(dest, char *, src->n_consoles);
-	for (i = 0; i < src->n_consoles && src->n_consoles; i++)
-		dest->consoles[i] = talloc_strdup(dest->consoles,
+		for (i = 0; i < src->n_consoles && src->n_consoles; i++)
+			if (src->consoles[i])
+				dest->consoles[i] = talloc_strdup(
+						dest->consoles,
 						src->consoles[i]);
+	}
 
 	if (src->boot_console)
 		dest->boot_console = talloc_strdup(dest, src->boot_console);
diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
index 33bd4e6..daf4ec9 100644
--- a/lib/pb-protocol/pb-protocol.c
+++ b/lib/pb-protocol/pb-protocol.c
@@ -99,13 +99,17 @@ int pb_protocol_serialise_string(char *pos, const char *str)
 {
 	int len = 0;
 
+	if (!pos)
+		return 0;
+
 	if (str)
 		len = strlen(str);
 
 	*(uint32_t *)pos = __cpu_to_be32(len);
 	pos += sizeof(uint32_t);
 
-	memcpy(pos, str, len);
+	if (str)
+		memcpy(pos, str, len);
 
 	return len + sizeof(uint32_t);
 }
@@ -417,9 +421,8 @@ int pb_protocol_serialise_device(const struct device *dev,
 	pos += pb_protocol_serialise_string(pos, dev->icon_file);
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_boot_option(const struct boot_option *opt,
@@ -447,9 +450,8 @@ int pb_protocol_serialise_boot_option(const struct boot_option *opt,
 	pos += 4;
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_boot_command(const struct boot_command *boot,
@@ -466,9 +468,8 @@ int pb_protocol_serialise_boot_command(const struct boot_command *boot,
 	pos += pb_protocol_serialise_string(pos, boot->console);
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_boot_status(const struct status *status,
@@ -488,9 +489,8 @@ int pb_protocol_serialise_boot_status(const struct status *status,
 	pos += sizeof(bool);
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_system_info(const struct system_info *sysinfo,
@@ -561,9 +561,8 @@ int pb_protocol_serialise_system_info(const struct system_info *sysinfo,
 	pos += HWADDR_SIZE;
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 static int pb_protocol_serialise_config_interface(char *buf,
@@ -669,9 +668,8 @@ int pb_protocol_serialise_config(const struct config *config,
 	pos += pb_protocol_serialise_string(pos, config->lang);
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_url(const char *url, char *buf, int buf_len)
@@ -681,9 +679,8 @@ int pb_protocol_serialise_url(const char *url, char *buf, int buf_len)
 	pos += pb_protocol_serialise_string(pos, url);
 
 	assert(pos <=buf+buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_plugin_option(const struct plugin_option *opt,
@@ -707,9 +704,8 @@ int pb_protocol_serialise_plugin_option(const struct plugin_option *opt,
 		pos += pb_protocol_serialise_string(pos, opt->executables[i]);
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_temp_autoboot(const struct autoboot_option *opt,
@@ -727,9 +723,9 @@ int pb_protocol_serialise_temp_autoboot(const struct autoboot_option *opt,
 		pos += pb_protocol_serialise_string(pos, opt->uuid);
 	}
 
-	(void)buf_len;
+	assert(pos <= buf + buf_len);
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_serialise_authenticate(struct auth_message *msg,
@@ -766,9 +762,8 @@ int pb_protocol_serialise_authenticate(struct auth_message *msg,
 	};
 
 	assert(pos <= buf + buf_len);
-	(void)buf_len;
 
-	return 0;
+	return (pos <= buf + buf_len) ? 0 : -1;
 }
 
 int pb_protocol_write_message(int fd, struct pb_protocol_message *message)
@@ -948,7 +943,7 @@ int pb_protocol_deserialise_boot_option(struct boot_option *opt,
 	if (read_u32(&pos, &len, &opt->type))
 		return -1;
 
-	rc = 0;
+	rc = (pos <= message->payload + message->payload_len) ? 0 : -1;
 
 out:
 	return rc;
diff --git a/ui/ncurses/nc-cui.c b/ui/ncurses/nc-cui.c
index bd2eb68..66f34b6 100644
--- a/ui/ncurses/nc-cui.c
+++ b/ui/ncurses/nc-cui.c
@@ -1052,7 +1052,9 @@ static int cui_boot_option_add(struct device *dev, struct boot_option *opt,
 		struct pmenu_item *item;
 		unsigned int j;
 		result = set_menu_items(cui->main->ncm, NULL);
-		for (j = 0 ; j < cui->main->item_count; j++) {
+		if (result)
+			pb_log_fn("unset_menu_items failed: %d\n", result);
+		for (j = 0 ; j < cui->main->item_count && !result; j++) {
 			item = item_userptr(cui->main->items[j]);
 			if (item->on_execute != menu_plugin_execute)
 				continue;
diff --git a/utils/hooks/30-dtb-updates.c b/utils/hooks/30-dtb-updates.c
index b8413fd..2d30c40 100644
--- a/utils/hooks/30-dtb-updates.c
+++ b/utils/hooks/30-dtb-updates.c
@@ -605,7 +605,7 @@ out:
 int main(void)
 {
 	struct offb_ctx *ctx;
-	int rc;
+	int rc = 0;
 
 	ctx = talloc_zero(NULL, struct offb_ctx);
 
-- 
cgit v1.2.1