diff mbox series

Various fixups and checks to make scan-build happy

Message ID 20190530052457.5413-1-sam@mendozajonas.com
State Accepted
Headers show
Series Various fixups and checks to make scan-build happy | expand

Commit Message

Sam Mendoza-Jonas May 30, 2019, 5:24 a.m. UTC 
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(-)

Comments

Sam Mendoza-Jonas June 11, 2019, 3:22 a.m. UTC | #1 
On Thu, 2019-05-30 at 15:24 +1000, Samuel Mendoza-Jonas wrote:
> 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(-)

Merged as 9e869eb

> 
> diff --git a/discover/device-handler.c b/discover/device-handler.c
> index d41bb4b0..d85f1afd 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 1a332cbf..e29ce272 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 ec5537a0..4c46d413 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 7eda0953..74d5729d 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 1a802b97..902df900 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 ae02bb0a..66b465e8 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 54b843e7..16fdd597 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 ba0f81c4..035794cd 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 d3d4a5e8..cc03ffd3 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 b06248f5..d0a40b1b 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 a802c92f..735cd989 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 33bd4e6e..daf4ec9d 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 bd2eb682..66f34b6e 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 b8413fd3..2d30c40c 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);
>
diff mbox series

Patch

diff --git a/discover/device-handler.c b/discover/device-handler.c
index d41bb4b0..d85f1afd 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 1a332cbf..e29ce272 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 ec5537a0..4c46d413 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 7eda0953..74d5729d 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 1a802b97..902df900 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 ae02bb0a..66b465e8 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 54b843e7..16fdd597 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 ba0f81c4..035794cd 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 d3d4a5e8..cc03ffd3 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 b06248f5..d0a40b1b 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 a802c92f..735cd989 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 33bd4e6e..daf4ec9d 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 bd2eb682..66f34b6e 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 b8413fd3..2d30c40c 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);