Merge pull request #11 from open-power/coverity

Coverity fixes

12 files changed, 40 insertions, 36 deletions

diff --git a/discover/device-handler.c b/discover/device-handler.c
index bfe5fff..3804b34 100644
--- a/discover/device-handler.c
+++ b/discover/device-handler.c
@@ -1472,7 +1472,6 @@ mount_ro:
 void device_release_write(struct discover_device *dev, bool release)
 {
 	const char *fstype, *device_path;
-	int rc = 0;
 
 	if (!release)
 		return;
@@ -1495,10 +1494,9 @@ void device_release_write(struct discover_device *dev, bool release)
 		device_path = get_device_path(dev);
 	}
 
-	mount(device_path, dev->mount_path, fstype,
+	if (mount(device_path, dev->mount_path, fstype,
 			MS_RDONLY | MS_SILENT,
-			fs_parameters(dev, MS_RDONLY));
-	if (rc)
+			fs_parameters(dev, MS_RDONLY)))
 		pb_log("Failed to remount %s read-only: %s\n",
 		       device_path, strerror(errno));
 	else
diff --git a/discover/dt.c b/discover/dt.c
index a7383e1..e7214e0 100644
--- a/discover/dt.c
+++ b/discover/dt.c
@@ -56,6 +56,7 @@ int get_ipmi_sensor(void *t, enum ipmi_sensor_ids sensor_id)
 	rc = __be32_to_cpu(*(uint32_t *)buf);
 
 out:
+	talloc_free(buf);
 	free(namelist);
 	return rc;
 }
diff --git a/discover/ipmi.c b/discover/ipmi.c
index 54d4e06..f59974a 100644
--- a/discover/ipmi.c
+++ b/discover/ipmi.c
@@ -183,7 +183,8 @@ int ipmi_transaction(struct ipmi *ipmi, uint8_t netfn, uint8_t cmd,
 
 out:
 	lock.l_type = F_UNLCK;
-	fcntl(ipmi->fd, F_SETLKW, &lock);
+	if (fcntl(ipmi->fd, F_SETLKW, &lock) == -1)
+		pb_log("IPMI: error unlocking IPMI device: %m\n");
 	return rc ? -1 : 0;
 }
 
diff --git a/discover/parser.c b/discover/parser.c
index 7833981..8e767c6 100644
--- a/discover/parser.c
+++ b/discover/parser.c
@@ -62,6 +62,7 @@ int parser_check_dir(struct discover_context *ctx,
 	path = local_path(ctx, dev, dirname);
 
 	rc = stat(path, &statbuf);
+	talloc_free(path);
 	if (!rc)
 		return -1;
 
diff --git a/discover/paths.c b/discover/paths.c
index 0d414eb..1269dde 100644
--- a/discover/paths.c
+++ b/discover/paths.c
@@ -6,6 +6,8 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 
 #include <talloc/talloc.h>
 #include <system/system.h>
@@ -49,9 +51,12 @@ char *join_paths(void *alloc_ctx, const char *a, const char *b)
 static char *local_name(void *ctx)
 {
 	char *ret, tmp[] = "/tmp/pb-XXXXXX";
+	mode_t oldmask;
 	int fd;
 
+	oldmask = umask(0644);
 	fd = mkstemp(tmp);
+	umask(oldmask);
 
 	if (fd < 0)
 		return NULL;
@@ -220,15 +225,16 @@ static enum tftp_type check_tftp_type(void *ctx)
 	const char *argv[] = { pb_system_apps.tftp, "-V", NULL };
 	struct process *process;
 	enum tftp_type type;
+	int rc;
 
 	process = process_create(ctx);
 	process->path = pb_system_apps.tftp;
 	process->argv = argv;
 	process->keep_stdout = true;
 	process->add_stderr = true;
-	process_run_sync(process);
+	rc = process_run_sync(process);
 
-	if (!process->stdout_buf || process->stdout_len == 0) {
+	if (rc || !process->stdout_buf || process->stdout_len == 0) {
 		pb_log("Can't check TFTP client type!\n");
 		type = TFTP_TYPE_BROKEN;
 
diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
index 2807934..7370d7d 100644
--- a/discover/platform-powerpc.c
+++ b/discover/platform-powerpc.c
@@ -179,7 +179,7 @@ static int write_nvram(struct platform_powerpc *platform)
 	struct process *process;
 	struct param *param;
 	const char *argv[6];
-	int rc;
+	int rc = 0;
 
 	argv[0] = "nvram";
 	argv[1] = "--update-config";
@@ -438,7 +438,6 @@ static void populate_bootdev_config(struct platform_powerpc *platform,
 {
 	struct autoboot_option *opt, *new = NULL;
 	char *pos, *end, *old_dev = NULL;
-	const char delim = ' ';
 	unsigned int n_new = 0;
 	const char *val;
 	bool conflict;
@@ -469,11 +468,9 @@ static void populate_bootdev_config(struct platform_powerpc *platform,
 
 		if (read_bootdev(config, &pos, opt)) {
 			pb_log("bootdev config is in an unknown format "
-			       "(expected uuid:... or mac:...)");
+			       "(expected uuid:... or mac:...)\n");
 			talloc_free(opt);
-			if (strchr(pos, delim))
-				continue;
-			return;
+			continue;
 		}
 
 		new = talloc_realloc(config, new, struct autoboot_option,
@@ -759,9 +756,8 @@ static int read_bootdev_sysparam(const char *name, uint8_t *val)
 	char path[50];
 	int fd, rc;
 
-	strcpy(path, sysparams_dir);
-	assert(strlen(name) < sizeof(path) - strlen(path));
-	strcat(path, name);
+	assert(strlen(sysparams_dir) + strlen(name) < sizeof(path));
+	snprintf(path, sizeof(path), "%s%s", sysparams_dir, name);
 
 	fd = open(path, O_RDONLY);
 	if (fd < 0) {
@@ -795,9 +791,8 @@ static int write_bootdev_sysparam(const char *name, uint8_t val)
 	char path[50];
 	int fd, rc;
 
-	strcpy(path, sysparams_dir);
-	assert(strlen(name) < sizeof(path) - strlen(path));
-	strcat(path, name);
+	assert(strlen(sysparams_dir) + strlen(name) < sizeof(path));
+	snprintf(path, sizeof(path), "%s%s", sysparams_dir, name);
 
 	fd = open(path, O_WRONLY);
 	if (fd < 0) {
diff --git a/discover/udev.c b/discover/udev.c
index 6cc718e..537ef10 100644
--- a/discover/udev.c
+++ b/discover/udev.c
@@ -245,13 +245,12 @@ static bool udev_handle_cdrom_events(struct pb_udev *udev,
 		struct udev_device *dev, struct discover_device *ddev)
 {
 	const char *node;
+	bool eject = false;
 
 	node = udev_device_get_devnode(dev);
 
 	/* handle CDROM eject requests */
 	if (udev_device_get_property_value(dev, "DISK_EJECT_REQUEST")) {
-		bool eject = false;
-
 		pb_debug("udev: eject request\n");
 
 		/* If the device is mounted, cdrom_id's own eject request may
@@ -260,7 +259,6 @@ static bool udev_handle_cdrom_events(struct pb_udev *udev,
 		if (ddev) {
 			eject = ddev->mounted;
 			udev_handle_dev_remove(udev, dev);
-			return false;
 		}
 
 		if (eject)
diff --git a/discover/user-event.c b/discover/user-event.c
index 69defa3..8cad1a9 100644
--- a/discover/user-event.c
+++ b/discover/user-event.c
@@ -488,10 +488,10 @@ static void user_event_handle_message(struct user_event *uev, char *buf,
 static int user_event_process(void *arg)
 {
 	struct user_event *uev = arg;
-	char buf[PBOOT_USER_EVENT_SIZE];
+	char buf[PBOOT_USER_EVENT_SIZE + 1];
 	int len;
 
-	len = recvfrom(uev->socket, buf, sizeof(buf), 0, NULL, NULL);
+	len = recvfrom(uev->socket, buf, PBOOT_USER_EVENT_SIZE, 0, NULL, NULL);
 
 	if (len < 0) {
 		pb_log("%s: socket read failed: %s", __func__, strerror(errno));
@@ -503,6 +503,8 @@ static int user_event_process(void *arg)
 		return 0;
 	}
 
+	buf[len] = '\0';
+
 	pb_debug("%s: %u bytes\n", __func__, len);
 
 	user_event_handle_message(uev, buf, len);
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index dd10893..fa34323 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -984,6 +984,7 @@ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
 		talloc_set_name_const(ret, ret);
 	}
 
+	va_end(ap2);
 	return ret;
 }
 
@@ -1038,6 +1039,7 @@ static char *talloc_vasprintf_append(char *s, const char *fmt, va_list ap)
 	vsnprintf(s+s_len, len+1, fmt, ap2);
 	talloc_set_name_const(s, s);
 
+	va_end(ap2);
 	return s;
 }
 
diff --git a/ui/common/discover-client.c b/ui/common/discover-client.c
index 14f36da..71ef70b 100644
--- a/ui/common/discover-client.c
+++ b/ui/common/discover-client.c
@@ -181,7 +181,7 @@ static int discover_client_process(void *arg)
 		rc = pb_protocol_deserialise_device(dev, message);
 		if (rc) {
 			pb_log("%s: no device?\n", __func__);
-			return 0;
+			goto out;
 		}
 
 		device_add(client, dev);
@@ -192,7 +192,7 @@ static int discover_client_process(void *arg)
 		rc = pb_protocol_deserialise_boot_option(opt, message);
 		if (rc) {
 			pb_log("%s: no boot_option?\n", __func__);
-			return 0;
+			goto out;
 		}
 
 		boot_option_add(client, opt);
@@ -201,7 +201,7 @@ static int discover_client_process(void *arg)
 		dev_id = pb_protocol_deserialise_string(ctx, message);
 		if (!dev_id) {
 			pb_log("%s: no device id?\n", __func__);
-			return 0;
+			goto out;
 		}
 		device_remove(client, dev_id);
 		break;
@@ -211,7 +211,7 @@ static int discover_client_process(void *arg)
 		rc = pb_protocol_deserialise_boot_status(status, message);
 		if (rc) {
 			pb_log("%s: invalid status message?\n", __func__);
-			return 0;
+			goto out;
 		}
 		update_status(client, status);
 		break;
@@ -221,7 +221,7 @@ static int discover_client_process(void *arg)
 		rc = pb_protocol_deserialise_system_info(sysinfo, message);
 		if (rc) {
 			pb_log("%s: invalid sysinfo message?\n", __func__);
-			return 0;
+			goto out;
 		}
 		update_sysinfo(client, sysinfo);
 		break;
@@ -231,7 +231,7 @@ static int discover_client_process(void *arg)
 		rc = pb_protocol_deserialise_config(config, message);
 		if (rc) {
 			pb_log("%s: invalid config message?\n", __func__);
-			return 0;
+			goto out;
 		}
 		update_config(client, config);
 		break;
@@ -239,6 +239,7 @@ static int discover_client_process(void *arg)
 		pb_log("%s: unknown action %d\n", __func__, message->action);
 	}
 
+out:
 	talloc_free(ctx);
 
 	return 0;
diff --git a/ui/ncurses/nc-menu.c b/ui/ncurses/nc-menu.c
index 3f09d62..dda7626 100644
--- a/ui/ncurses/nc-menu.c
+++ b/ui/ncurses/nc-menu.c
@@ -322,9 +322,10 @@ static int pmenu_item_get_index(const struct pmenu_item *item)
 {
 	unsigned int i;
 
-	for (i = 0; i < item->pmenu->item_count; i++)
-		if (item->pmenu->items[i] == item->nci)
-			return i;
+	if (item)
+		for (i = 0; i < item->pmenu->item_count; i++)
+			if (item->pmenu->items[i] == item->nci)
+				return i;
 
 	pb_log("%s: not found: %p %s\n", __func__, item,
 		(item ? item->nci->name.str : "(null)"));
diff --git a/ui/ncurses/nc-widgets.c b/ui/ncurses/nc-widgets.c
index 7e03e57..98d30cc 100644
--- a/ui/ncurses/nc-widgets.c
+++ b/ui/ncurses/nc-widgets.c
@@ -1224,10 +1224,8 @@ void widgetset_post(struct nc_widgetset *set)
 	post_form(set->form);
 	form_driver(set->form, REQ_END_FIELD);
 
-	if (set->cur_field) {
+	if (set->cur_field)
 		set_current_field(set->form, set->cur_field);
-		field = set->cur_field;
-	}
 
 	field = current_field(set->form);
 	widget = field_userptr(field);