[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-hackers
Subject:    Re: [PATCH] mfiutil(8) - capture errors and percolate up to caller
From:       Garrett Cooper <yanegomi () gmail ! com>
Date:       2010-10-26 18:52:16
Message-ID: AANLkTimPPdHPGpfUThjeA8rmhFmrLz94DggVhkgSa=E7 () mail ! gmail ! com
[Download RAW message or body]

On Tue, Oct 26, 2010 at 11:38 AM, John Baldwin <jhb@freebsd.org> wrote:
> On Tuesday, October 26, 2010 2:09:53 pm Garrett Cooper wrote:
>>     Because a number of places in the mfiutil(8) code immediately call
>> warn(3) after an error to an API occurred, and because warn(3) employs
>> printf, et all (multiple times) in libc, there's an off-chance that
>> the errno value can get stomped on by the warn(3) calls, which could
>> lead to confusing results from anyone depending on the value being
>> returned from the mfiutil APIs. Thus, the attached patch I'm providing
>> fixes those cases, as well as converts an existing internal API
>> (display_pending_firmware) to an non-void return mechanism. I also
>> made a few stack variable alignment changes to match style(9) as well
>> as got rid of the ad hoc powerof2 call in favor of the value in
>> sys/param.h.
>>     I've run a small number of unit tests on my desktop at home with
>> my mfi(4) card, but will test out other failing cases with equipment I
>> have access to at work.
>
> Just a few nits:
>
> 1) The include of <sys/param.h> should replace <sys/types.h> (there's a note
> about these two headers in style(9), FYI).

Done -- thanks for pointing me to that :).

> 2) patrol_get_props() should return 'error' on failure rather than 'errno'.

Good catch -- fixed!

> 3) mfi_get_time() failing isn't fatal.  The code already handles this case by
> not printing out a 'next run time' if at is zero.  I think you can remove the
> check for at == 0.  If all the other commands work and just that command fails
> I don't think it should be fatal.

Ok -- removed the at == 0 check.

Thanks!
-Garrett

["mfiutil-capture-errno-values.patch" (text/x-patch)]

Index: usr.sbin/mfiutil/mfi_config.c
===================================================================
--- usr.sbin/mfiutil/mfi_config.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_config.c	(working copy)
@@ -29,12 +29,12 @@
  * $FreeBSD$
  */
 
-#include <sys/types.h>
+#include <sys/param.h>
 #ifdef DEBUG
 #include <sys/sysctl.h>
 #endif
-#include <sys/errno.h>
 #include <err.h>
+#include <errno.h>
 #include <libutil.h>
 #ifdef DEBUG
 #include <stdint.h>
@@ -52,8 +52,6 @@
 static int	add_spare(int ac, char **av);
 static int	remove_spare(int ac, char **av);
 
-#define powerof2(x)    ((((x)-1)&(x))==0)
-
 static long
 dehumanize(const char *value)
 {
@@ -151,13 +149,14 @@
 clear_config(int ac, char **av)
 {
 	struct mfi_ld_list list;
-	int ch, fd;
+	int ch, error, fd;
 	u_int i;
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (!mfi_reconfig_supported()) {
@@ -167,8 +166,9 @@
 	}
 
 	if (mfi_ld_get_list(fd, &list, NULL) < 0) {
+		error = errno;
 		warn("Failed to get volume list");
-		return (errno);
+		return (error);
 	}
 
 	for (i = 0; i < list.ld_count; i++) {
@@ -189,8 +189,9 @@
 	}
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_CLEAR, NULL, 0, NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to clear configuration");
-		return (errno);
+		return (error);
 	}
 
 	printf("mfi%d: Configuration cleared\n", mfi_unit);
@@ -335,8 +336,9 @@
 			return (error);
 
 		if (mfi_pd_get_info(fd, device_id, pinfo, NULL) < 0) {
+			error = errno;
 			warn("Failed to fetch drive info for drive %s", cp);
-			return (errno);
+			return (error);
 		}
 
 		if (pinfo->fw_state != MFI_PD_STATE_UNCONFIGURED_GOOD) {
@@ -548,8 +550,9 @@
 	
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (!mfi_reconfig_supported()) {
@@ -660,8 +663,9 @@
 	 * array and volume identifiers.
 	 */
 	if (mfi_config_read(fd, &config) < 0) {
+		error = errno;
 		warn("Failed to read configuration");
-		return (errno);
+		return (error);
 	}
 	p = (char *)config->array;
 	state.array_ref = 0xffff;
@@ -745,14 +749,14 @@
 #ifdef DEBUG
 	if (dump)
 		dump_config(fd, config);
-	else
 #endif
 
 	/* Send the new config to the controller. */
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_ADD, config, config_size,
 	    NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to add volume");
-		return (errno);
+		return (error);
 	}
 
 	/* Clean up. */
@@ -774,7 +778,7 @@
 delete_volume(int ac, char **av)
 {
 	struct mfi_ld_info info;
-	int fd;
+	int error, fd;
 	uint8_t target_id, mbox[4];
 
 	/*
@@ -799,8 +803,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (!mfi_reconfig_supported()) {
@@ -810,13 +815,15 @@
 	}
 
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
+		error = errno;
 		warn("Invalid volume %s", av[1]);
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_ld_get_info(fd, target_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to get info for volume %d", target_id);
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_volume_busy(fd, target_id)) {
@@ -828,8 +835,9 @@
 	mbox_store_ldref(mbox, &info.ld_config.properties.ld);
 	if (mfi_dcmd_command(fd, MFI_DCMD_LD_DELETE, NULL, 0, mbox,
 	    sizeof(mbox), NULL) < 0) {
+		error = errno;
 		warn("Failed to delete volume");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -858,8 +866,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -867,8 +876,9 @@
 		return (error);
 
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch drive info");
-		return (errno);
+		return (error);
 	}
 
 	if (info.fw_state != MFI_PD_STATE_UNCONFIGURED_GOOD) {
@@ -878,14 +888,16 @@
 
 	if (ac > 2) {
 		if (mfi_lookup_volume(fd, av[2], &target_id) < 0) {
+			error = errno;
 			warn("Invalid volume %s", av[2]);
-			return (errno);
+			return (error);
 		}
 	}
 
 	if (mfi_config_read(fd, &config) < 0) {
+		error = errno;
 		warn("Failed to read configuration");
-		return (errno);
+		return (error);
 	}
 
 	spare = malloc(sizeof(struct mfi_spare) + sizeof(uint16_t) *
@@ -939,8 +951,9 @@
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_MAKE_SPARE, spare,
 	    sizeof(struct mfi_spare) + sizeof(uint16_t) * spare->array_count,
 	    NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to assign spare");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -964,8 +977,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -974,8 +988,9 @@
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 
 	if (info.fw_state != MFI_PD_STATE_HOT_SPARE) {
@@ -986,8 +1001,9 @@
 	mbox_store_pdref(mbox, &info.ref);
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_REMOVE_SPARE, NULL, 0, mbox,
 	    sizeof(mbox), NULL) < 0) {
+		error = errno;
 		warn("Failed to delete spare");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -1093,7 +1109,7 @@
 debug_config(int ac, char **av)
 {
 	struct mfi_config_data *config;
-	int fd;
+	int error, fd;
 
 	if (ac != 1) {
 		warnx("debug: extra arguments");
@@ -1102,14 +1118,16 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	/* Get the config from the controller. */
 	if (mfi_config_read(fd, &config) < 0) {
+		error = errno;
 		warn("Failed to get config");
-		return (errno);
+		return (error);
 	}
 
 	/* Dump out the configuration. */
@@ -1127,7 +1145,7 @@
 	struct mfi_config_data *config;
 	char buf[64];
 	size_t len;
-	int fd;
+	int error, fd;
 
 	if (ac != 1) {
 		warnx("dump: extra arguments");
@@ -1136,23 +1154,26 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	/* Get the stashed copy of the last dcmd from the driver. */
 	snprintf(buf, sizeof(buf), "dev.mfi.%d.debug_command", mfi_unit);
 	if (sysctlbyname(buf, NULL, &len, NULL, 0) < 0) {
+		error = errno;
 		warn("Failed to read debug command");
-		if (errno == ENOENT)
-			errno = EOPNOTSUPP;
-		return (errno);
+		if (error == ENOENT)
+			error = EOPNOTSUPP;
+		return (error);
 	}
 
 	config = malloc(len);
 	if (sysctlbyname(buf, config, &len, NULL, 0) < 0) {
+		error = errno;
 		warn("Failed to read debug command");
-		return (errno);
+		return (error);
 	}
 	dump_config(fd, config);
 	free(config);
Index: usr.sbin/mfiutil/mfi_volume.c
===================================================================
--- usr.sbin/mfiutil/mfi_volume.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_volume.c	(working copy)
@@ -114,6 +114,7 @@
 update_cache_policy(int fd, struct mfi_ld_props *props, uint8_t new_policy,
     uint8_t mask)
 {
+	int error;
 	uint8_t changes, policy;
 
 	policy = (props->default_cache_policy & ~mask) | new_policy;
@@ -140,8 +141,9 @@
 
 	props->default_cache_policy = policy;
 	if (mfi_ld_set_props(fd, props) < 0) {
+		error = errno;
 		warn("Failed to set volume properties");
-		return (errno);
+		return (error);
 	}
 	return (0);
 }
@@ -160,18 +162,21 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
+		error = errno;
 		warn("Invalid volume: %s", av[1]);
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_ld_get_props(fd, target_id, &props) < 0) {
+		error = errno;
 		warn("Failed to fetch volume properties");
-		return (errno);
+		return (error);
 	}
 
 	if (ac == 2) {
@@ -298,8 +303,8 @@
 				}
 				props.disk_cache_policy = policy;
 				if (mfi_ld_set_props(fd, &props) < 0) {
+					error = errno;
 					warn("Failed to set volume properties");
-					error = errno;
 				}
 			}
 		} else {
@@ -317,7 +322,7 @@
 volume_name(int ac, char **av)
 {
 	struct mfi_ld_props props;
-	int fd;
+	int error, fd;
 	uint8_t target_id;
 
 	if (ac != 3) {
@@ -332,18 +337,21 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
+		error = errno;
 		warn("Invalid volume: %s", av[1]);
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_ld_get_props(fd, target_id, &props) < 0) {
+		error = errno;
 		warn("Failed to fetch volume properties");
-		return (errno);
+		return (error);
 	}
 
 	printf("mfi%u volume %s name changed from \"%s\" to \"%s\"\n", mfi_unit,
@@ -351,8 +359,9 @@
 	bzero(props.name, sizeof(props.name));
 	strcpy(props.name, av[2]);
 	if (mfi_ld_set_props(fd, &props) < 0) {
+		error = errno;
 		warn("Failed to set volume properties");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -365,7 +374,7 @@
 volume_progress(int ac, char **av)
 {
 	struct mfi_ld_info info;
-	int fd;
+	int error, fd;
 	uint8_t target_id;
 
 	if (ac != 2) {
@@ -376,20 +385,23 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
+		error = errno;
 		warn("Invalid volume: %s", av[1]);
-		return (errno);
+		return (error);
 	}
 
 	/* Get the info for this drive. */
 	if (mfi_ld_get_info(fd, target_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for volume %s",
 		    mfi_volume_name(fd, target_id));
-		return (errno);
+		return (error);
 	}
 
 	/* Display any of the active events. */
Index: usr.sbin/mfiutil/mfi_drive.c
===================================================================
--- usr.sbin/mfiutil/mfi_drive.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_drive.c	(working copy)
@@ -79,10 +79,11 @@
 mfi_lookup_drive(int fd, char *drive, uint16_t *device_id)
 {
 	struct mfi_pd_list *list;
-	uint8_t encl, slot;
 	long val;
+	int error;
 	u_int i;
 	char *cp;
+	uint8_t encl, slot;
 
 	/* Look for a raw device id first. */
 	val = strtol(drive, &cp, 0);
@@ -118,8 +119,9 @@
 		slot = val;
 
 		if (mfi_pd_get_list(fd, &list, NULL) < 0) {
+			error = errno;
 			warn("Failed to fetch drive list");
-			return (errno);
+			return (error);
 		}
 
 		for (i = 0; i < list->count; i++) {
@@ -302,8 +304,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, drive, &device_id);
@@ -312,8 +315,9 @@
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 
 	/* Try to change the state. */
@@ -327,9 +331,10 @@
 	mbox[5] = new_state >> 8;
 	if (mfi_dcmd_command(fd, MFI_DCMD_PD_STATE_SET, NULL, 0, mbox, 6,
 	    NULL) < 0) {
+		error = errno;
 		warn("Failed to set drive %u to %s", device_id,
 		    mfi_pdstate(new_state));
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -395,8 +400,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -405,13 +411,14 @@
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 
 	/* Check the state, must be REBUILD. */
 	if (info.fw_state != MFI_PD_STATE_REBUILD) {
-		warn("Drive %d is not in the REBUILD state", device_id);
+		warnx("Drive %d is not in the REBUILD state", device_id);
 		return (EINVAL);
 	}
 
@@ -419,8 +426,9 @@
 	mbox_store_pdref(&mbox[0], &info.ref);
 	if (mfi_dcmd_command(fd, MFI_DCMD_PD_REBUILD_START, NULL, 0, mbox, 4,
 	    NULL) < 0) {
+		error = errno;
 		warn("Failed to start rebuild on drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 	close(fd);
 
@@ -444,8 +452,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -454,8 +463,9 @@
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 
 	/* Check the state, must be REBUILD. */
@@ -468,8 +478,9 @@
 	mbox_store_pdref(&mbox[0], &info.ref);
 	if (mfi_dcmd_command(fd, MFI_DCMD_PD_REBUILD_ABORT, NULL, 0, mbox, 4,
 	    NULL) < 0) {
+		error = errno;
 		warn("Failed to abort rebuild on drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 	close(fd);
 
@@ -492,8 +503,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -502,8 +514,9 @@
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 	close(fd);
 
@@ -551,8 +564,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -561,16 +575,18 @@
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
-		return (errno);
+		return (error);
 	}
 
 	mbox_store_pdref(&mbox[0], &info.ref);
 	if (mfi_dcmd_command(fd, opcode, NULL, 0, mbox, 4, NULL) < 0) {
+		error = errno;
 		warn("Failed to %s clear on drive %u",
 		    opcode == MFI_DCMD_PD_CLEAR_START ? "start" : "stop",
 		    device_id);
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -604,8 +620,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
@@ -617,10 +634,11 @@
 	mbox[2] = 0;
 	mbox[3] = 0;
 	if (mfi_dcmd_command(fd, opcode, NULL, 0, mbox, 4, NULL) < 0) {
+		error = errno;
 		warn("Failed to %s locate on drive %u",
 		    opcode == MFI_DCMD_PD_LOCATE_START ? "start" : "stop",
 		    device_id);
-		return (errno);
+		return (error);
 	}
 	close(fd);
 
Index: usr.sbin/mfiutil/mfi_flash.c
===================================================================
--- usr.sbin/mfiutil/mfi_flash.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_flash.c	(working copy)
@@ -72,16 +72,18 @@
 	    fw_time_width, comp->build_time);
 }
 
-static void
+static int
 display_pending_firmware(int fd)
 {
 	struct mfi_ctrl_info info;
 	struct mfi_info_component header;
+	int error;
 	u_int i;
 
 	if (mfi_ctrl_get_info(fd, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to get controller info");
-		return;
+		return (error);
 	}
 
 	printf("mfi%d Pending Firmware Images:\n", mfi_unit);
@@ -97,6 +99,8 @@
 	display_firmware(&header);
 	for (i = 0; i < info.pending_image_component_count; i++)
 		display_firmware(&info.pending_image_component[i]);
+
+	return (0);
 }
 
 static void
@@ -117,7 +121,7 @@
 	size_t nread;
 	char *buf;
 	struct stat sb;
-	int fd, flash;
+	int error, fd, flash;
 	uint8_t mbox[4], status;
 
 	if (ac != 2) {
@@ -127,13 +131,15 @@
 
 	flash = open(av[1], O_RDONLY);
 	if (flash < 0) {
+		error = errno;
 		warn("flash: Failed to open %s", av[1]);
-		return (errno);
+		return (error);
 	}
 
 	if (fstat(flash, &sb) < 0) {
+		error = errno;
 		warn("fstat(%s)", av[1]);
-		return (errno);
+		return (error);
 	}
 	if (sb.st_size % 1024 != 0 || sb.st_size > 0x7fffffff) {
 		warnx("Invalid flash file size");
@@ -142,8 +148,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	/* First, ask the firmware to allocate space for the flash file. */
@@ -190,10 +197,10 @@
 		return (ENXIO);
 	}
 	printf("finished\n");
-	display_pending_firmware(fd);
+	error = display_pending_firmware(fd);
 
 	close(fd);
 
-	return (0);
+	return (error);
 }
 MFI_COMMAND(top, flash, flash_adapter);
Index: usr.sbin/mfiutil/mfi_evt.c
===================================================================
--- usr.sbin/mfiutil/mfi_evt.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_evt.c	(working copy)
@@ -32,7 +32,6 @@
 #include <sys/types.h>
 #include <sys/errno.h>
 #include <err.h>
-//#include <libutil.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <strings.h>
@@ -67,7 +66,7 @@
 show_logstate(int ac, char **av)
 {
 	struct mfi_evt_log_state info;
-	int fd;
+	int error, fd;
 
 	if (ac != 1) {
 		warnx("show logstate: extra arguments");
@@ -76,13 +75,15 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_event_get_info(fd, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to get event log info");
-		return (errno);
+		return (error);
 	}
 
 	printf("mfi%d Event Log Sequence Numbers:\n", mfi_unit);
@@ -536,18 +537,20 @@
 	ssize_t size;
 	uint32_t seq, start, stop;
 	uint8_t status;
-	int ch, fd, num_events, verbose;
+	int ch, error, fd, num_events, verbose;
 	u_int i;
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_event_get_info(fd, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to get event log info");
-		return (errno);
+		return (error);
 	}
 
 	/* Default settings. */
@@ -565,14 +568,16 @@
 		switch (ch) {
 		case 'c':
 			if (parse_class(optarg, &filter.members.class) < 0) {
+				error = errno;
 				warn("Error parsing event class");
-				return (errno);
+				return (error);
 			}
 			break;
 		case 'l':
 			if (parse_locale(optarg, &filter.members.locale) < 0) {
+				error = errno;
 				warn("Error parsing event locale");
-				return (errno);
+				return (error);
 			}
 			break;
 		case 'n':
@@ -608,20 +613,23 @@
 		return (EINVAL);
 	}
 	if (ac > 0 && parse_seq(&info, av[0], &start) < 0) {
+		error = errno;
 		warn("Error parsing starting sequence number");
-		return (errno);
+		return (error);
 	}
 	if (ac > 1 && parse_seq(&info, av[1], &stop) < 0) {
+		error = errno;
 		warn("Error parsing ending sequence number");
-		return (errno);
+		return (error);
 	}
 
 	list = malloc(size);
 	for (seq = start;;) {
 		if (mfi_get_events(fd, list, num_events, filter, seq,
 		    &status) < 0) {
+			error = errno;
 			warn("Failed to fetch events");
-			return (errno);
+			return (error);
 		}
 		if (status == MFI_STAT_NOT_FOUND) {
 			if (seq == start)
Index: usr.sbin/mfiutil/mfi_show.c
===================================================================
--- usr.sbin/mfiutil/mfi_show.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_show.c	(working copy)
@@ -54,7 +54,7 @@
 {
 	struct mfi_ctrl_info info;
 	char stripe[5];
-	int fd, comma;
+	int error, fd, comma;
 
 	if (ac != 1) {
 		warnx("show adapter: extra arguments");
@@ -63,13 +63,15 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_ctrl_get_info(fd, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to get controller info");
-		return (errno);
+		return (error);
 	}
 	printf("mfi%d Adapter:\n", mfi_unit);
 	printf("    Product Name: %.80s\n", info.product_name);
@@ -137,7 +139,7 @@
 	struct mfi_bbu_capacity_info cap;
 	struct mfi_bbu_design_info design;
 	uint8_t status;
-	int fd;
+	int error, fd;
 
 	if (ac != 1) {
 		warnx("show battery: extra arguments");
@@ -146,8 +148,9 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_BBU_GET_CAPACITY_INFO, &cap,
@@ -156,14 +159,16 @@
 			printf("mfi%d: No battery present\n", mfi_unit);
 			return (0);
 		}
+		error = errno;
 		warn("Failed to get capacity info");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_BBU_GET_DESIGN_INFO, &design,
 	    sizeof(design), NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to get design info");
-		return (errno);
+		return (error);
 	}
 
 	printf("mfi%d: Battery State:\n", mfi_unit);
@@ -242,7 +247,7 @@
 	struct mfi_pd_info pinfo;
 	uint16_t device_id;
 	char *p;
-	int fd, i, j;
+	int error, fd, i, j;
 
 	if (ac != 1) {
 		warnx("show config: extra arguments");
@@ -251,14 +256,16 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	/* Get the config from the controller. */
 	if (mfi_config_read(fd, &config) < 0) {
+		error = errno;
 		warn("Failed to get config");
-		return (errno);
+		return (error);
 	}
 
 	/* Dump out the configuration. */
@@ -337,8 +344,8 @@
 {
 	struct mfi_ld_list list;
 	struct mfi_ld_info info;
+	int error, fd;
 	u_int i, len, state_len;
-	int fd;
 
 	if (ac != 1) {
 		warnx("show volumes: extra arguments");
@@ -347,14 +354,16 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	/* Get the logical drive list from the controller. */
 	if (mfi_ld_get_list(fd, &list, NULL) < 0) {
+		error = errno;
 		warn("Failed to get volume list");
-		return (errno);
+		return (error);
 	}
 
 	/* List the volumes. */
@@ -376,9 +385,10 @@
 	for (i = 0; i < list.ld_count; i++) {
 		if (mfi_ld_get_info(fd, list.ld_list[i].ld.v.target_id, &info,
 		    NULL) < 0) {
+			error = errno;
 			warn("Failed to get info for volume %d",
 			    list.ld_list[i].ld.v.target_id);
-			return (errno);
+			return (error);
 		}
 		printf("%6s ",
 		    mfi_volume_name(fd, list.ld_list[i].ld.v.target_id));
@@ -416,7 +426,7 @@
 	struct mfi_pd_list *list;
 	struct mfi_pd_info info;
 	u_int i, len, state_len;
-	int fd;
+	int error, fd;
 
 	if (ac != 1) {
 		warnx("show drives: extra arguments");
@@ -425,13 +435,15 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_pd_get_list(fd, &list, NULL) < 0) {
+		error = errno;
 		warn("Failed to get drive list");
-		return (errno);
+		return (error);
 	}
 
 	/* Walk the list of drives to determine width of state column. */
@@ -442,9 +454,10 @@
 
 		if (mfi_pd_get_info(fd, list->addr[i].device_id, &info,
 		    NULL) < 0) {
+			error = errno;
 			warn("Failed to fetch info for drive %u",
 			    list->addr[i].device_id);
-			return (errno);
+			return (error);
 		}
 		len = strlen(mfi_pdstate(info.fw_state));
 		if (len > state_len)
@@ -462,9 +475,10 @@
 		/* Fetch details for this drive. */
 		if (mfi_pd_get_info(fd, list->addr[i].device_id, &info,
 		    NULL) < 0) {
+			error = errno;
 			warn("Failed to fetch info for drive %u",
 			    list->addr[i].device_id);
-			return (errno);
+			return (error);
 		}
 
 		print_pd(&info, state_len, 1);
@@ -511,7 +525,7 @@
 {
 	struct mfi_ctrl_info info;
 	struct mfi_info_component header;
-	int fd;
+	int error, fd;
 	u_int i;
 
 	if (ac != 1) {
@@ -521,13 +535,15 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_ctrl_get_info(fd, &info, NULL) < 0) {
+		error = errno;
 		warn("Failed to get controller info");
-		return (errno);
+		return (error);
 	}
 
 	if (info.package_version[0] != '\0')
Index: usr.sbin/mfiutil/mfi_patrol.c
===================================================================
--- usr.sbin/mfiutil/mfi_patrol.c	(revision 214260)
+++ usr.sbin/mfiutil/mfi_patrol.c	(working copy)
@@ -62,11 +62,13 @@
 static int
 patrol_get_props(int fd, struct mfi_pr_properties *prop)
 {
+	int error;
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_PR_GET_PROPERTIES, prop,
 	    sizeof(*prop), NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to get patrol read properties");
-		return (-1);
+		return (error);
 	}
 	return (0);
 }
@@ -81,19 +83,21 @@
 	char label[16];
 	time_t now;
 	uint32_t at;
-	int fd;
+	int error, fd;
 	u_int i;
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	time(&now);
 	mfi_get_time(fd, &at);
-	if (patrol_get_props(fd, &prop) < 0)
-		return (errno);
+	error = patrol_get_props(fd, &prop);
+	if (error)
+		return (error);
 	printf("Operation Mode: ");
 	switch (prop.op_mode) {
 	case MFI_PR_OPMODE_AUTO:
@@ -122,8 +126,9 @@
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_PR_GET_STATUS, &status,
 	    sizeof(status), NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to get patrol read properties");
-		return (errno);
+		return (error);
 	}
 	printf("Runs Completed: %u\n", status.num_iteration);
 	printf("Current State: ");
@@ -146,8 +151,9 @@
 	}
 	if (status.state == MFI_PR_STATE_ACTIVE) {
 		if (mfi_pd_get_list(fd, &list, NULL) < 0) {
+			error = errno;
 			warn("Failed to get drive list");
-			return (errno);
+			return (error);
 		}
 
 		for (i = 0; i < list->count; i++) {
@@ -156,9 +162,10 @@
 
 			if (mfi_pd_get_info(fd, list->addr[i].device_id, &info,
 			    NULL) < 0) {
+				error = errno;
 				warn("Failed to fetch info for drive %u",
 				    list->addr[i].device_id);
-				return (errno);
+				return (error);
 			}
 			if (info.prog_info.active & MFI_PD_PROGRESS_PATROL) {
 				snprintf(label, sizeof(label), "    Drive %u",
@@ -178,18 +185,20 @@
 static int
 start_patrol(int ac, char **av)
 {
-	int fd;
+	int error, fd;
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_PR_START, NULL, 0, NULL, 0, NULL) <
 	    0) {
+		error = errno;
 		warn("Failed to start patrol read");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -201,18 +210,20 @@
 static int
 stop_patrol(int ac, char **av)
 {
-	int fd;
+	int error, fd;
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_PR_STOP, NULL, 0, NULL, 0, NULL) <
 	    0) {
+		error = errno;
 		warn("Failed to stop patrol read");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);
@@ -227,10 +238,10 @@
 	struct mfi_pr_properties prop;
 	long val;
 	time_t now;
+	int error, fd;
 	uint32_t at, next_exec, exec_freq;
 	char *cp;
 	uint8_t op_mode;
-	int fd;
 
 	exec_freq = 0;	/* GCC too stupid */
 	next_exec = 0;
@@ -272,12 +283,14 @@
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
+		error = errno;
 		warn("mfi_open");
-		return (errno);
+		return (error);
 	}
 
-	if (patrol_get_props(fd, &prop) < 0)
-		return (errno);
+	error = patrol_get_props(fd, &prop);
+	if (error)
+		return (error);
 	prop.op_mode = op_mode;
 	if (op_mode == MFI_PR_OPMODE_AUTO) {
 		if (ac > 2)
@@ -294,8 +307,9 @@
 	}
 	if (mfi_dcmd_command(fd, MFI_DCMD_PR_SET_PROPERTIES, &prop,
 	    sizeof(prop), NULL, 0, NULL) < 0) {
+		error = errno;
 		warn("Failed to set patrol read properties");
-		return (errno);
+		return (error);
 	}
 
 	close(fd);


_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic