[PATCH] Various fixups and checks to make scan-build happy
Samuel Mendoza-Jonas
sam at mendozajonas.com
Tue Jun 11 13:22:08 AEST 2019
On Thu, 2019-05-30 at 15:24 +1000, Samuel Mendoza-Jonas wrote:
> Signed-off-by: Samuel Mendoza-Jonas <sam at 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);
>
More information about the Petitboot
mailing list