Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[patch] give gpioctl.c some love



I was troubleshooting a GPIO problem and came across usr.sbin/gpioctl/gpioctl.c. The code is well-written, but I figured it needs some love, readability-wise.  It uses unnecessarily deep nesting while trying to stay witin 80 characters per line at tabstop 8; I refactored parts of it to eliminate that problem (maintaining the 80-colums invariant, of course).

Apart from that, a few other things were changed here and there:
 - const correctness
 - meaningful prototypes
 - avoidance of redundancies
 - fix (and isolate) converting a string to `unsigned` (it errorneously accepted some negative values before, and didn't zero/restore errno (not that it has restore, but it certainly is good manners to do)
 - declutter main() a bit

Overall, a patch that removes (slightly) more lines than it adds, so it must be good, no? :)
I'll inline it here for easier commenting, if there are objections.


Cheers,

Timo Buhrmester

P.S.: gpioctl's functionality or behavior is not changed, apart from the integer parsing issue (a missing `lval < 0`)


----------8<---------------------------------------------------------

--- usr.sbin/gpioctl/gpioctl.c.orig
+++ usr.sbin/gpioctl/gpioctl.c
@@ -36,17 +36,21 @@
 #include <string.h>
 #include <unistd.h>
 
-static char *dev;
+static const char *dev;
 static int devfd = -1;
 static int quiet = 0;
 static int state = 0;
 
 static void getinfo(void);
-static void gpioread(int, char *);
-static void gpiowrite(int, char *, int);
-static void gpioset(int pin, char *name, int flags, char *alias);
-static void gpiounset(int pin, char *name);
-static void devattach(char *, int, uint32_t, uint32_t);
+static void gpioread(int pin, const char *gp_name);
+static void gpiowrite(int pin, const char *gp_name, int value);
+static void gpioset(int pin, const char *name, int flags, const char *alias);
+static void gpiounset(int pin, const char *name);
+
+static unsigned get_uint_or_die(const char *str, const char *name);
+static void devattach(const char *driver, const char *offset,
+    const char *mask, const char *flags);
+
 __dead static void usage(void);
 
 static const struct bitstr {
@@ -67,19 +71,14 @@ static const struct bitstr {
 	{ 0, NULL },
 };
 
+
 int
 main(int argc, char *argv[])
 {
 	const struct bitstr *bs;
 	int pin, ch, n, fl = 0, value = 0;
 	const char *errstr;
-	char *ep;
-	int ga_offset = -1;
-	uint32_t ga_mask = 0;
-	uint32_t ga_flags = 0;
-	long lval;
 	char *nam = NULL;
-	char *flags;
 	char devn[32];
 
 	while ((ch = getopt(argc, argv, "qs")) != -1)
@@ -115,84 +114,54 @@ main(int argc, char *argv[])
 	}
 
 	if (!strcmp(argv[1], "attach")) {
-		char *driver, *offset, *mask;
-
 		if (argc != 5 && argc != 6)
 			usage();
 
-		driver = argv[2];
-		offset = argv[3];
-		mask = argv[4];
-		flags = argc == 6 ? argv[5] : NULL;
-
-		ga_offset = strtonum(offset, 0, INT_MAX, &errstr);
-		if (errstr)
-			errx(EXIT_FAILURE, "offset is %s: %s", errstr, offset);
-		lval = strtol(mask, &ep, 0);
-		if (*mask == '\0' || *ep != '\0')
-			errx(EXIT_FAILURE, "invalid mask (not a number)");
-		if ((errno == ERANGE && (lval == LONG_MAX
-		    || lval == LONG_MIN)) || (unsigned long)lval > UINT_MAX)
-			errx(EXIT_FAILURE, "mask out of range");
-		ga_mask = lval;
-		if (flags != NULL) {
-			lval = strtol(flags, &ep, 0);
-			if (*flags == '\0' || *ep != '\0')
-				errx(EXIT_FAILURE,
-				    "invalid flag locator (not a number)");
-			if ((errno == ERANGE && (lval == LONG_MAX
-			    || lval == LONG_MIN))
-			    || (unsigned long)lval > UINT_MAX)
-				errx(EXIT_FAILURE, "flag locator out of range");
-
-			ga_flags = lval;
-		}
-		devattach(driver, ga_offset, ga_mask, ga_flags);
+		devattach(argv[2], argv[3], argv[4], argv[5]);
+
 		return EXIT_SUCCESS;
-	} else {
-		char *nm = NULL;
-
-		/* expecting a pin number or name */
-		pin = strtonum(argv[1], 0, INT_MAX, &errstr);
-		if (errstr)
-			nm = argv[1];	/* try named pin */
-		if (argc > 2) {
-			if (!strcmp(argv[2], "set")) {
-				for (n = 3; n < argc; n++) {
-					for (bs = pinflags; bs->string != NULL;
-					     bs++) {
-						if (!strcmp(argv[n],
-						    bs->string)) {
-							fl |= bs->mask;
-							break;
-						}
-					}
-					if (bs->string == NULL)
-						nam = argv[n];
-				}
-				gpioset(pin, nm, fl, nam);
-			} else if (!strcmp(argv[2], "unset"))
-				gpiounset(pin, nm);
-			else {
-				value = strtonum(argv[2], INT_MIN, INT_MAX,
-				   &errstr);
-				if (errstr) {
-					if (!strcmp(argv[2], "on"))
-						value = GPIO_PIN_HIGH;
-					else if (!strcmp(argv[2], "off"))
-						value = GPIO_PIN_LOW;
-					else if (!strcmp(argv[2], "toggle"))
-						value = 2;
-					else
-						errx(EXIT_FAILURE,
-						    "%s: invalid value",
-						    argv[2]);
+	}
+
+	char *nm = NULL;
+	/* expecting a pin number or name */
+	pin = strtonum(argv[1], 0, INT_MAX, &errstr);
+	if (errstr)
+		nm = argv[1];	/* try named pin */
+
+	if (argc <= 2) {
+		gpioread(pin, nm);
+		return EXIT_SUCCESS;
+	}
+
+	if (!strcmp(argv[2], "set")) {
+		for (n = 3; n < argc; n++) {
+			for (bs = pinflags; bs->string; bs++) {
+				if (!strcmp(argv[n], bs->string)) {
+					fl |= bs->mask;
+					break;
 				}
-				gpiowrite(pin, nm, value);
 			}
-		} else
-			gpioread(pin, nm);
+			if (!bs->string)
+				nam = argv[n];
+		}
+		gpioset(pin, nm, fl, nam);
+	} else if (!strcmp(argv[2], "unset")) {
+		gpiounset(pin, nm);
+	} else {
+		value = strtonum(argv[2], INT_MIN, INT_MAX, &errstr);
+		if (errstr) {
+			if (!strcmp(argv[2], "on"))
+				value = GPIO_PIN_HIGH;
+			else if (!strcmp(argv[2], "off"))
+				value = GPIO_PIN_LOW;
+			else if (!strcmp(argv[2], "toggle"))
+				value = 2;
+			else
+				errx(EXIT_FAILURE, "%s: invalid value",argv[2]);
+		}
+		gpiowrite(pin, nm, value);
 	}
+
 	return EXIT_SUCCESS;
 }
 
@@ -214,12 +183,12 @@ getinfo(void)
 }
 
 static void
-gpioread(int pin, char *gp_name)
+gpioread(int pin, const char *gp_name)
 {
 	struct gpio_req req;
 
 	memset(&req, 0, sizeof(req));
-	if (gp_name != NULL)
+	if (gp_name)
 		strlcpy(req.gp_name, gp_name, sizeof(req.gp_name));
 	else
 		req.gp_pin = pin;
@@ -240,7 +209,7 @@ gpioread(int pin, char *gp_name)
 }
 
 static void
-gpiowrite(int pin, char *gp_name, int value)
+gpiowrite(int pin, const char *gp_name, int value)
 {
 	struct gpio_req req;
 
@@ -248,7 +217,7 @@ gpiowrite(int pin, char *gp_name, int value)
 		errx(EXIT_FAILURE, "%d: invalid value", value);
 
 	memset(&req, 0, sizeof(req));
-	if (gp_name != NULL)
+	if (gp_name)
 		strlcpy(req.gp_name, gp_name, sizeof(req.gp_name));
 	else
 		req.gp_pin = pin;
@@ -257,10 +226,8 @@ gpiowrite(int pin, char *gp_name, int value)
 		req.gp_value = value;
 		if (ioctl(devfd, GPIOWRITE, &req) == -1)
 			err(EXIT_FAILURE, "GPIOWRITE");
-	} else {
-		if (ioctl(devfd, GPIOTOGGLE, &req) == -1)
-			err(EXIT_FAILURE, "GPIOTOGGLE");
-	}
+	} else if (ioctl(devfd, GPIOTOGGLE, &req) == -1)
+		err(EXIT_FAILURE, "GPIOTOGGLE");
 
 	if (state)
 		printf("%d\n", value < 2 ? value : 1 - req.gp_value);
@@ -277,19 +244,19 @@ gpiowrite(int pin, char *gp_name, int value)
 }
 
 static void
-gpioset(int pin, char *name, int fl, char *alias)
+gpioset(int pin, const char *name, int fl, const char *alias)
 {
 	struct gpio_set set;
 	const struct bitstr *bs;
 
 	memset(&set, 0, sizeof(set));
-	if (name != NULL)
+	if (name)
 		strlcpy(set.gp_name, name, sizeof(set.gp_name));
 	else
 		set.gp_pin = pin;
 	set.gp_flags = fl;
 
-	if (alias != NULL)
+	if (alias)
 		strlcpy(set.gp_name2, alias, sizeof(set.gp_name2));
 
 	if (ioctl(devfd, GPIOSET, &set) == -1)
@@ -298,20 +265,20 @@ gpioset(int pin, char *name, int fl, char *alias)
 	if (quiet)
 		return;
 
-	if (name != NULL)
+	if (name)
 		printf("pin %s: caps:", name);
 	else
 		printf("pin %d: caps:", pin);
-	for (bs = pinflags; bs->string != NULL; bs++)
+	for (bs = pinflags; bs->string; bs++)
 		if (set.gp_caps & bs->mask)
 			printf(" %s", bs->string);
 	printf(", flags:");
-	for (bs = pinflags; bs->string != NULL; bs++)
+	for (bs = pinflags; bs->string; bs++)
 		if (set.gp_flags & bs->mask)
 			printf(" %s", bs->string);
 	if (fl > 0) {
 		printf(" ->");
-		for (bs = pinflags; bs->string != NULL; bs++)
+		for (bs = pinflags; bs->string; bs++)
 			if (fl & bs->mask)
 				printf(" %s", bs->string);
 	}
@@ -319,12 +286,12 @@ gpioset(int pin, char *name, int fl, char *alias)
 }
 
 static void
-gpiounset(int pin, char *name)
+gpiounset(int pin, const char *name)
 {
 	struct gpio_set set;
 
 	memset(&set, 0, sizeof(set));
-	if (name != NULL)
+	if (name)
 		strlcpy(set.gp_name, name, sizeof(set.gp_name));
 	else
 		set.gp_pin = pin;
@@ -333,16 +300,45 @@ gpiounset(int pin, char *name)
 		err(EXIT_FAILURE, "GPIOUNSET");
 }
 
+static unsigned
+get_uint_or_die(const char *str, const char *name)
+{
+	int oerrno;
+	char *ep;
+
+	long lval = strtol(str, &ep, 0);
+	if (!*str || *ep)
+		errx(EXIT_FAILURE, "invalid %s (not a number)", name);
+
+	oerrno = errno, errno = 0;
+
+	if ((errno == ERANGE && (lval == LONG_MAX || lval == LONG_MIN))
+	    || lval < 0 || (unsigned long)lval > UINT_MAX)
+		errx(EXIT_FAILURE, "%s out of range", name);
+
+	errno = oerrno;
+
+	return (unsigned)lval;
+}
+
 static void
-devattach(char *dvname, int offset, uint32_t mask, uint32_t flags)
+devattach(const char *driver, const char *offset,
+    const char *mask, const char *flags)
 {
-	struct gpio_attach attach;
+	const char *errstr;
+	struct gpio_attach attach = { .ga_flags = 0 };
+
+	attach.ga_offset = strtonum(offset, 0, INT_MAX, &errstr);
+	if (errstr)
+		errx(EXIT_FAILURE, "offset is %s: %s", errstr, offset);
+
+	attach.ga_mask = get_uint_or_die(mask, "mask");
+
+	if (flags)
+		attach.ga_flags = get_uint_or_die(flags, "flags");
+
+	strlcpy(attach.ga_dvname, driver, sizeof(attach.ga_dvname));
 
-	memset(&attach, 0, sizeof(attach));
-	strlcpy(attach.ga_dvname, dvname, sizeof(attach.ga_dvname));
-	attach.ga_offset = offset;
-	attach.ga_mask = mask;
-	attach.ga_flags = flags;
 	if (ioctl(devfd, GPIOATTACH, &attach) == -1)
 		err(EXIT_FAILURE, "GPIOATTACH");
 }
@@ -350,17 +346,14 @@ devattach(char *dvname, int offset, uint32_t mask, uint32_t flags)
 static void
 usage(void)
 {
-	const char *progname;
-
-	progname = getprogname();
-	fprintf(stderr, "usage: %s [-qs] device [pin] [0 | 1 | 2 | "
-	    "on | off | toggle]\n", progname);
-	fprintf(stderr, "       %s [-q] device pin set [flags] [name]\n",
-	    progname);
-	fprintf(stderr, "       %s [-q] device pin unset\n", progname);
-	fprintf(stderr, "       %s [-q] device attach device offset mask "
-	    "[flag]\n",
-	    progname);
+	const char *pn = getprogname();
+
+	#define U(...) fprintf(stderr, __VA_ARGS__)
+	U("usage: %s [-qs] device [pin] [0 | 1 | 2 | on | off | toggle]\n", pn);
+	U("       %s [-q] device pin set [flags] [name]\n", pn);
+	U("       %s [-q] device pin unset\n", pn);
+	U("       %s [-q] device attach device offset mask [flag]\n", pn);
+	#undef U
 
 	exit(EXIT_FAILURE);
 }
--- usr.sbin/gpioctl/gpioctl.c.orig
+++ usr.sbin/gpioctl/gpioctl.c
@@ -36,17 +36,21 @@
 #include <string.h>
 #include <unistd.h>
 
-static char *dev;
+static const char *dev;
 static int devfd = -1;
 static int quiet = 0;
 static int state = 0;
 
 static void getinfo(void);
-static void gpioread(int, char *);
-static void gpiowrite(int, char *, int);
-static void gpioset(int pin, char *name, int flags, char *alias);
-static void gpiounset(int pin, char *name);
-static void devattach(char *, int, uint32_t, uint32_t);
+static void gpioread(int pin, const char *gp_name);
+static void gpiowrite(int pin, const char *gp_name, int value);
+static void gpioset(int pin, const char *name, int flags, const char *alias);
+static void gpiounset(int pin, const char *name);
+
+static unsigned get_uint_or_die(const char *str, const char *name);
+static void devattach(const char *driver, const char *offset,
+    const char *mask, const char *flags);
+
 __dead static void usage(void);
 
 static const struct bitstr {
@@ -67,19 +71,14 @@ static const struct bitstr {
 	{ 0, NULL },
 };
 
+
 int
 main(int argc, char *argv[])
 {
 	const struct bitstr *bs;
 	int pin, ch, n, fl = 0, value = 0;
 	const char *errstr;
-	char *ep;
-	int ga_offset = -1;
-	uint32_t ga_mask = 0;
-	uint32_t ga_flags = 0;
-	long lval;
 	char *nam = NULL;
-	char *flags;
 	char devn[32];
 
 	while ((ch = getopt(argc, argv, "qs")) != -1)
@@ -115,84 +114,54 @@ main(int argc, char *argv[])
 	}
 
 	if (!strcmp(argv[1], "attach")) {
-		char *driver, *offset, *mask;
-
 		if (argc != 5 && argc != 6)
 			usage();
 
-		driver = argv[2];
-		offset = argv[3];
-		mask = argv[4];
-		flags = argc == 6 ? argv[5] : NULL;
-
-		ga_offset = strtonum(offset, 0, INT_MAX, &errstr);
-		if (errstr)
-			errx(EXIT_FAILURE, "offset is %s: %s", errstr, offset);
-		lval = strtol(mask, &ep, 0);
-		if (*mask == '\0' || *ep != '\0')
-			errx(EXIT_FAILURE, "invalid mask (not a number)");
-		if ((errno == ERANGE && (lval == LONG_MAX
-		    || lval == LONG_MIN)) || (unsigned long)lval > UINT_MAX)
-			errx(EXIT_FAILURE, "mask out of range");
-		ga_mask = lval;
-		if (flags != NULL) {
-			lval = strtol(flags, &ep, 0);
-			if (*flags == '\0' || *ep != '\0')
-				errx(EXIT_FAILURE,
-				    "invalid flag locator (not a number)");
-			if ((errno == ERANGE && (lval == LONG_MAX
-			    || lval == LONG_MIN))
-			    || (unsigned long)lval > UINT_MAX)
-				errx(EXIT_FAILURE, "flag locator out of range");
-
-			ga_flags = lval;
-		}
-		devattach(driver, ga_offset, ga_mask, ga_flags);
+		devattach(argv[2], argv[3], argv[4], argv[5]);
+
 		return EXIT_SUCCESS;
-	} else {
-		char *nm = NULL;
-
-		/* expecting a pin number or name */
-		pin = strtonum(argv[1], 0, INT_MAX, &errstr);
-		if (errstr)
-			nm = argv[1];	/* try named pin */
-		if (argc > 2) {
-			if (!strcmp(argv[2], "set")) {
-				for (n = 3; n < argc; n++) {
-					for (bs = pinflags; bs->string != NULL;
-					     bs++) {
-						if (!strcmp(argv[n],
-						    bs->string)) {
-							fl |= bs->mask;
-							break;
-						}
-					}
-					if (bs->string == NULL)
-						nam = argv[n];
-				}
-				gpioset(pin, nm, fl, nam);
-			} else if (!strcmp(argv[2], "unset"))
-				gpiounset(pin, nm);
-			else {
-				value = strtonum(argv[2], INT_MIN, INT_MAX,
-				   &errstr);
-				if (errstr) {
-					if (!strcmp(argv[2], "on"))
-						value = GPIO_PIN_HIGH;
-					else if (!strcmp(argv[2], "off"))
-						value = GPIO_PIN_LOW;
-					else if (!strcmp(argv[2], "toggle"))
-						value = 2;
-					else
-						errx(EXIT_FAILURE,
-						    "%s: invalid value",
-						    argv[2]);
+	}
+
+	char *nm = NULL;
+	/* expecting a pin number or name */
+	pin = strtonum(argv[1], 0, INT_MAX, &errstr);
+	if (errstr)
+		nm = argv[1];	/* try named pin */
+
+	if (argc <= 2) {
+		gpioread(pin, nm);
+		return EXIT_SUCCESS;
+	}
+
+	if (!strcmp(argv[2], "set")) {
+		for (n = 3; n < argc; n++) {
+			for (bs = pinflags; bs->string; bs++) {
+				if (!strcmp(argv[n], bs->string)) {
+					fl |= bs->mask;
+					break;
 				}
-				gpiowrite(pin, nm, value);
 			}
-		} else
-			gpioread(pin, nm);
+			if (!bs->string)
+				nam = argv[n];
+		}
+		gpioset(pin, nm, fl, nam);
+	} else if (!strcmp(argv[2], "unset")) {
+		gpiounset(pin, nm);
+	} else {
+		value = strtonum(argv[2], INT_MIN, INT_MAX, &errstr);
+		if (errstr) {
+			if (!strcmp(argv[2], "on"))
+				value = GPIO_PIN_HIGH;
+			else if (!strcmp(argv[2], "off"))
+				value = GPIO_PIN_LOW;
+			else if (!strcmp(argv[2], "toggle"))
+				value = 2;
+			else
+				errx(EXIT_FAILURE, "%s: invalid value",argv[2]);
+		}
+		gpiowrite(pin, nm, value);
 	}
+
 	return EXIT_SUCCESS;
 }
 
@@ -214,12 +183,12 @@ getinfo(void)
 }
 
 static void
-gpioread(int pin, char *gp_name)
+gpioread(int pin, const char *gp_name)
 {
 	struct gpio_req req;
 
 	memset(&req, 0, sizeof(req));
-	if (gp_name != NULL)
+	if (gp_name)
 		strlcpy(req.gp_name, gp_name, sizeof(req.gp_name));
 	else
 		req.gp_pin = pin;
@@ -240,7 +209,7 @@ gpioread(int pin, char *gp_name)
 }
 
 static void
-gpiowrite(int pin, char *gp_name, int value)
+gpiowrite(int pin, const char *gp_name, int value)
 {
 	struct gpio_req req;
 
@@ -248,7 +217,7 @@ gpiowrite(int pin, char *gp_name, int value)
 		errx(EXIT_FAILURE, "%d: invalid value", value);
 
 	memset(&req, 0, sizeof(req));
-	if (gp_name != NULL)
+	if (gp_name)
 		strlcpy(req.gp_name, gp_name, sizeof(req.gp_name));
 	else
 		req.gp_pin = pin;
@@ -257,10 +226,8 @@ gpiowrite(int pin, char *gp_name, int value)
 		req.gp_value = value;
 		if (ioctl(devfd, GPIOWRITE, &req) == -1)
 			err(EXIT_FAILURE, "GPIOWRITE");
-	} else {
-		if (ioctl(devfd, GPIOTOGGLE, &req) == -1)
-			err(EXIT_FAILURE, "GPIOTOGGLE");
-	}
+	} else if (ioctl(devfd, GPIOTOGGLE, &req) == -1)
+		err(EXIT_FAILURE, "GPIOTOGGLE");
 
 	if (state)
 		printf("%d\n", value < 2 ? value : 1 - req.gp_value);
@@ -277,19 +244,19 @@ gpiowrite(int pin, char *gp_name, int value)
 }
 
 static void
-gpioset(int pin, char *name, int fl, char *alias)
+gpioset(int pin, const char *name, int fl, const char *alias)
 {
 	struct gpio_set set;
 	const struct bitstr *bs;
 
 	memset(&set, 0, sizeof(set));
-	if (name != NULL)
+	if (name)
 		strlcpy(set.gp_name, name, sizeof(set.gp_name));
 	else
 		set.gp_pin = pin;
 	set.gp_flags = fl;
 
-	if (alias != NULL)
+	if (alias)
 		strlcpy(set.gp_name2, alias, sizeof(set.gp_name2));
 
 	if (ioctl(devfd, GPIOSET, &set) == -1)
@@ -298,20 +265,20 @@ gpioset(int pin, char *name, int fl, char *alias)
 	if (quiet)
 		return;
 
-	if (name != NULL)
+	if (name)
 		printf("pin %s: caps:", name);
 	else
 		printf("pin %d: caps:", pin);
-	for (bs = pinflags; bs->string != NULL; bs++)
+	for (bs = pinflags; bs->string; bs++)
 		if (set.gp_caps & bs->mask)
 			printf(" %s", bs->string);
 	printf(", flags:");
-	for (bs = pinflags; bs->string != NULL; bs++)
+	for (bs = pinflags; bs->string; bs++)
 		if (set.gp_flags & bs->mask)
 			printf(" %s", bs->string);
 	if (fl > 0) {
 		printf(" ->");
-		for (bs = pinflags; bs->string != NULL; bs++)
+		for (bs = pinflags; bs->string; bs++)
 			if (fl & bs->mask)
 				printf(" %s", bs->string);
 	}
@@ -319,12 +286,12 @@ gpioset(int pin, char *name, int fl, char *alias)
 }
 
 static void
-gpiounset(int pin, char *name)
+gpiounset(int pin, const char *name)
 {
 	struct gpio_set set;
 
 	memset(&set, 0, sizeof(set));
-	if (name != NULL)
+	if (name)
 		strlcpy(set.gp_name, name, sizeof(set.gp_name));
 	else
 		set.gp_pin = pin;
@@ -333,16 +300,45 @@ gpiounset(int pin, char *name)
 		err(EXIT_FAILURE, "GPIOUNSET");
 }
 
+static unsigned
+get_uint_or_die(const char *str, const char *name)
+{
+	int oerrno;
+	char *ep;
+
+	long lval = strtol(str, &ep, 0);
+	if (!*str || *ep)
+		errx(EXIT_FAILURE, "invalid %s (not a number)", name);
+
+	oerrno = errno, errno = 0;
+
+	if ((errno == ERANGE && (lval == LONG_MAX || lval == LONG_MIN))
+	    || lval < 0 || (unsigned long)lval > UINT_MAX)
+		errx(EXIT_FAILURE, "%s out of range", name);
+
+	errno = oerrno;
+
+	return (unsigned)lval;
+}
+
 static void
-devattach(char *dvname, int offset, uint32_t mask, uint32_t flags)
+devattach(const char *driver, const char *offset,
+    const char *mask, const char *flags)
 {
-	struct gpio_attach attach;
+	const char *errstr;
+	struct gpio_attach attach = { .ga_flags = 0 };
+
+	attach.ga_offset = strtonum(offset, 0, INT_MAX, &errstr);
+	if (errstr)
+		errx(EXIT_FAILURE, "offset is %s: %s", errstr, offset);
+
+	attach.ga_mask = get_uint_or_die(mask, "mask");
+
+	if (flags)
+		attach.ga_flags = get_uint_or_die(flags, "flags");
+
+	strlcpy(attach.ga_dvname, driver, sizeof(attach.ga_dvname));
 
-	memset(&attach, 0, sizeof(attach));
-	strlcpy(attach.ga_dvname, dvname, sizeof(attach.ga_dvname));
-	attach.ga_offset = offset;
-	attach.ga_mask = mask;
-	attach.ga_flags = flags;
 	if (ioctl(devfd, GPIOATTACH, &attach) == -1)
 		err(EXIT_FAILURE, "GPIOATTACH");
 }
@@ -350,17 +346,14 @@ devattach(char *dvname, int offset, uint32_t mask, uint32_t flags)
 static void
 usage(void)
 {
-	const char *progname;
-
-	progname = getprogname();
-	fprintf(stderr, "usage: %s [-qs] device [pin] [0 | 1 | 2 | "
-	    "on | off | toggle]\n", progname);
-	fprintf(stderr, "       %s [-q] device pin set [flags] [name]\n",
-	    progname);
-	fprintf(stderr, "       %s [-q] device pin unset\n", progname);
-	fprintf(stderr, "       %s [-q] device attach device offset mask "
-	    "[flag]\n",
-	    progname);
+	const char *pn = getprogname();
+
+	#define U(...) fprintf(stderr, __VA_ARGS__)
+	U("usage: %s [-qs] device [pin] [0 | 1 | 2 | on | off | toggle]\n", pn);
+	U("       %s [-q] device pin set [flags] [name]\n", pn);
+	U("       %s [-q] device pin unset\n", pn);
+	U("       %s [-q] device attach device offset mask [flag]\n", pn);
+	#undef U
 
 	exit(EXIT_FAILURE);
 }


Home | Main Index | Thread Index | Old Index