git: b23362afa995 - main - Final pass of cleanup: Get rid of gotos and general polish.

Poul-Henning Kamp phk at FreeBSD.org
Thu May 13 12:00:30 UTC 2021


The branch main has been updated by phk:

URL: https://cgit.FreeBSD.org/src/commit/?id=b23362afa9956a22225dc4edd5dd4d5883e39de8

commit b23362afa9956a22225dc4edd5dd4d5883e39de8
Author:     Poul-Henning Kamp <phk at FreeBSD.org>
AuthorDate: 2021-05-13 11:58:50 +0000
Commit:     Poul-Henning Kamp <phk at FreeBSD.org>
CommitDate: 2021-05-13 11:58:50 +0000

    Final pass of cleanup: Get rid of gotos and general polish.
    
    NB: This changes the outputs, in particular in error conditions,
        in various subtle ways to be more systematic and usable.
---
 usr.sbin/i2c/i2c.c | 361 ++++++++++++++++++++++++-----------------------------
 1 file changed, 162 insertions(+), 199 deletions(-)

diff --git a/usr.sbin/i2c/i2c.c b/usr.sbin/i2c/i2c.c
index 753ddbf712d3..ef0ca0e8fda5 100644
--- a/usr.sbin/i2c/i2c.c
+++ b/usr.sbin/i2c/i2c.c
@@ -53,7 +53,7 @@ __FBSDID("$FreeBSD$");
 
 struct options {
 	const char	*width;
-	int		count;
+	unsigned	count;
 	int		verbose;
 	int		binary;
 	const char	*skip;
@@ -80,6 +80,80 @@ usage(const char *msg)
 	exit(EX_USAGE);
 }
 
+static int
+i2c_do_stop(int fd)
+{
+	int i;
+
+	i = ioctl(fd, I2CSTOP);
+	if (i < 0)
+		fprintf(stderr, "ioctl: error sending stop condition: %s\n",
+		    strerror(errno));
+	return (i);
+}
+
+static int
+i2c_do_start(int fd, struct iiccmd *cmd)
+{
+	int i;
+
+	i = ioctl(fd, I2CSTART, cmd);
+	if (i < 0)
+		fprintf(stderr, "ioctl: error sending start condition: %s\n",
+		    strerror(errno));
+	return (i);
+}
+
+static int
+i2c_do_repeatstart(int fd, struct iiccmd *cmd)
+{
+	int i;
+
+	i = ioctl(fd, I2CRPTSTART, cmd);
+	if (i < 0)
+		fprintf(stderr, "ioctl: error sending repeated start: %s\n",
+		    strerror(errno));
+	return (i);
+}
+
+static int
+i2c_do_write(int fd, struct iiccmd *cmd)
+{
+	int i;
+
+	i = ioctl(fd, I2CWRITE, cmd);
+	if (i < 0)
+		fprintf(stderr, "ioctl: error writing: %s\n",
+		    strerror(errno));
+	return (i);
+}
+
+static int
+i2c_do_read(int fd, struct iiccmd *cmd)
+{
+	int i;
+
+	i = ioctl(fd, I2CREAD, cmd);
+	if (i < 0)
+		fprintf(stderr, "ioctl: error reading: %s\n",
+		    strerror(errno));
+	return (i);
+}
+
+static int
+i2c_do_reset(int fd)
+{
+	struct iiccmd cmd;
+	int i;
+
+	memset(&cmd, 0, sizeof cmd);
+	i = ioctl(fd, I2CRSTCARD, &cmd);
+	if (i < 0)
+		fprintf(stderr, "ioctl: error resetting controller: %s\n",
+		    strerror(errno));
+	return (i);
+}
+
 static void
 parse_skip(const char *skip, char blacklist[128])
 {
@@ -130,7 +204,7 @@ parse_skip(const char *skip, char blacklist[128])
 }
 
 static int
-scan_bus(const char *dev, int fd, const char *skip)
+scan_bus(const char *dev, int fd, const char *skip, int verbose)
 {
 	struct iiccmd cmd;
 	struct iic_msg rdmsg;
@@ -140,14 +214,13 @@ scan_bus(const char *dev, int fd, const char *skip)
 	int num_found = 0, use_read_xfer;
 	uint8_t rdbyte;
 	char blacklist[128];
+	const char *sep = "";
 
 	memset(blacklist, 0, sizeof blacklist);
 
 	if (skip != NULL)
 		parse_skip(skip, blacklist);
 
-	printf("Scanning I2C devices on %s:", dev);
-
 	for (use_read_xfer = 0; use_read_xfer < 2; use_read_xfer++) {
 		for (u = 1; u < 127; u++) {
 			if (blacklist[u])
@@ -156,14 +229,9 @@ scan_bus(const char *dev, int fd, const char *skip)
 			cmd.slave = u << 1;
 			cmd.last = 1;
 			cmd.count = 0;
-			error = ioctl(fd, I2CRSTCARD, &cmd);
-			if (error) {
-				fprintf(stderr, "Controller reset failed\n");
-				fprintf(stderr,
-				    "Error scanning I2C controller (%s): %s\n",
-				    dev, strerror(errno));
+			if (i2c_do_reset(fd))
 				return (EX_NOINPUT);
-			}
+
 			if (use_read_xfer) {
 				rdmsg.buf = &rdbyte;
 				rdmsg.len = 1;
@@ -173,51 +241,43 @@ scan_bus(const char *dev, int fd, const char *skip)
 				rdwrdata.nmsgs = 1;
 				error = ioctl(fd, I2CRDWR, &rdwrdata);
 			} else {
-				cmd.slave = u << 1;
-				cmd.last = 1;
 				error = ioctl(fd, I2CSTART, &cmd);
 				if (errno == ENODEV || errno == EOPNOTSUPP)
 					break; /* Try reads instead */
 				(void)ioctl(fd, I2CSTOP);
 			}
 			if (error == 0) {
-				++num_found;
-				printf(" %02x", u);
+				if (!num_found++ && verbose) {
+					fprintf(stderr,
+					    "Scanning I2C devices on %s:\n",
+					    dev);
+				}
+				printf("%s%02x", sep, u);
+				sep = " ";
 			}
 		}
 		if (num_found > 0)
 			break;
-		fprintf(stderr,
-		    "Hardware may not support START/STOP scanning; "
-		    "trying less-reliable read method.\n");
+		if (verbose && !use_read_xfer)
+			fprintf(stderr,
+			    "Hardware may not support START/STOP scanning; "
+			    "trying less-reliable read method.\n");
 	}
-	if (num_found == 0)
-		printf("<none found>");
+	if (num_found == 0 && verbose)
+		printf("<Nothing Found>");
 
 	printf("\n");
 
-	error = ioctl(fd, I2CRSTCARD, &cmd);
-	if (error)
-		fprintf(stderr, "Controller reset failed\n");
-	return (EX_OK);
+	return (i2c_do_reset(fd));
 }
 
 static int
-reset_bus(const char *dev, int fd)
+reset_bus(const char *dev, int fd, int verbose)
 {
-	struct iiccmd cmd;
-	int error;
-
-	printf("Resetting I2C controller on %s: ", dev);
-	error = ioctl(fd, I2CRSTCARD, &cmd);
 
-	if (error) {
-		printf("error: %s\n", strerror(errno));
-		return (EX_IOERR);
-	} else {
-		printf("OK\n");
-		return (EX_OK);
-	}
+	if (verbose)
+		fprintf(stderr, "Resetting I2C controller on %s\n", dev);
+	return (i2c_do_reset(fd));
 }
 
 static const char *
@@ -250,214 +310,127 @@ encode_offset(const char *width, unsigned address, uint8_t *dst, size_t *len)
 	return ("Invalid offset width, must be: 0|8|16|16LE|16BE\n");
 }
 
-static const char *
+static int
 write_offset(int fd, struct options i2c_opt, struct iiccmd *cmd)
 {
-	int error;
 
 	if (i2c_opt.off_len > 0) {
 		cmd->count = i2c_opt.off_len;
 		cmd->buf = (void*)i2c_opt.off_buf;
-		error = ioctl(fd, I2CWRITE, cmd);
-		if (error == -1)
-			return ("ioctl: error writing offset\n");
+		return (i2c_do_write(fd, cmd));
 	}
-	return (NULL);
+	return (0);
 }
 
 static int
-i2c_write(int fd, struct options i2c_opt, char *i2c_buf)
+i2c_write(int fd, struct options i2c_opt, uint8_t *i2c_buf)
 {
 	struct iiccmd cmd;
-	int error;
-	char *buf;
-	const char *err_msg;
+	char buf[i2c_opt.off_len + i2c_opt.count];
 
+	memset(&cmd, 0, sizeof(cmd));
 	cmd.slave = i2c_opt.addr;
-	error = ioctl(fd, I2CSTART, &cmd);
-	if (error == -1) {
-		err_msg = "ioctl: error sending start condition";
-		goto err1;
-	}
+
+	if (i2c_do_start(fd, &cmd))
+		return (i2c_do_stop(fd) | 1);
 
 	switch(i2c_opt.mode) {
 	case I2C_MODE_STOP_START:
-		err_msg = write_offset(fd, i2c_opt, &cmd);
-		if (err_msg != NULL)
-			goto err1;
-
-		error = ioctl(fd, I2CSTOP);
-		if (error == -1) {
-			err_msg = "ioctl: error sending stop condition";
-			goto err2;
-		}
-		cmd.slave = i2c_opt.addr;
-		error = ioctl(fd, I2CSTART, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error sending start condition";
-			goto err1;
-		}
+		if (write_offset(fd, i2c_opt, &cmd))
+			return (i2c_do_stop(fd) | 1);
+
+		if (i2c_do_stop(fd))
+			return (1);
+
+		if (i2c_do_start(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 
 		/*
 		 * Write the data
 		 */
 		cmd.count = i2c_opt.count;
-		cmd.buf = i2c_buf;
+		cmd.buf = (void*)i2c_buf;
 		cmd.last = 0;
-		error = ioctl(fd, I2CWRITE, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error writing";
-			goto err1;
-		}
+		if (i2c_do_write(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 		break;
 
 	case I2C_MODE_REPEATED_START:
-		err_msg = write_offset(fd, i2c_opt, &cmd);
-		if (err_msg != NULL)
-			goto err1;
-
-		cmd.slave = i2c_opt.addr;
-		error = ioctl(fd, I2CRPTSTART, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error sending repeated start "
-			    "condition";
-			goto err1;
-		}
+		if (write_offset(fd, i2c_opt, &cmd))
+			return (i2c_do_stop(fd) | 1);
+
+		if (i2c_do_repeatstart(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 
 		/*
 		 * Write the data
 		 */
 		cmd.count = i2c_opt.count;
-		cmd.buf = i2c_buf;
+		cmd.buf = (void*)i2c_buf;
 		cmd.last = 0;
-		error = ioctl(fd, I2CWRITE, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error writing";
-			goto err1;
-		}
+		if (i2c_do_write(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 		break;
 
 	case I2C_MODE_NONE: /* fall through */
 	default:
-		buf = malloc(i2c_opt.off_len + i2c_opt.count);
-		if (buf == NULL) {
-			err_msg = "error: data malloc";
-			goto err1;
-		}
 		memcpy(buf, i2c_opt.off_buf, i2c_opt.off_len);
-
 		memcpy(buf + i2c_opt.off_len, i2c_buf, i2c_opt.count);
 		/*
 		 * Write offset and data
 		 */
 		cmd.count = i2c_opt.off_len + i2c_opt.count;
-		cmd.buf = buf;
+		cmd.buf = (void*)buf;
 		cmd.last = 0;
-		error = ioctl(fd, I2CWRITE, &cmd);
-		free(buf);
-		if (error == -1) {
-			err_msg = "ioctl: error writing";
-			goto err1;
-		}
+		if (i2c_do_write(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 		break;
 	}
-	error = ioctl(fd, I2CSTOP);
-	if (error == -1) {
-		err_msg = "ioctl: error sending stop condition";
-		goto err2;
-	}
-
-	return (0);
-
-err1:
-	error = ioctl(fd, I2CSTOP);
-	if (error == -1)
-		fprintf(stderr, "error sending stop condition\n");
-err2:
-	if (err_msg)
-		fprintf(stderr, "%s\n", err_msg);
 
-	return (1);
+	return (i2c_do_stop(fd));
 }
 
 static int
-i2c_read(int fd, struct options i2c_opt, char *i2c_buf)
+i2c_read(int fd, struct options i2c_opt, uint8_t *i2c_buf)
 {
 	struct iiccmd cmd;
-	int error;
 	char data = 0;
-	const char *err_msg;
 
-	bzero(&cmd, sizeof(cmd));
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.slave = i2c_opt.addr;
 
 	if (i2c_opt.off_len) {
-		cmd.slave = i2c_opt.addr;
 		cmd.count = 1;
 		cmd.last = 0;
 		cmd.buf = &data;
-		error = ioctl(fd, I2CSTART, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error sending start condition";
-			goto err1;
-		}
+		if (i2c_do_start(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 
-		err_msg = write_offset(fd, i2c_opt, &cmd);
-		if (err_msg != NULL)
-			goto err1;
+		if (write_offset(fd, i2c_opt, &cmd))
+			return (i2c_do_stop(fd) | 1);
 
-		if (i2c_opt.mode == I2C_MODE_STOP_START) {
-			error = ioctl(fd, I2CSTOP);
-			if (error == -1) {
-				err_msg = "error sending stop condition";
-				goto err2;
-			}
-		}
+		if (i2c_opt.mode == I2C_MODE_STOP_START && i2c_do_stop(fd))
+			return (1);
 	}
 	cmd.slave = i2c_opt.addr | 1;
 	cmd.count = 1;
 	cmd.last = 0;
 	cmd.buf = &data;
 	if (i2c_opt.mode == I2C_MODE_STOP_START || i2c_opt.off_len == 0) {
-		error = ioctl(fd, I2CSTART, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error sending start condition";
-			goto err2;
-		}
+		if (i2c_do_start(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 	} else if (i2c_opt.mode == I2C_MODE_REPEATED_START) {
-		error = ioctl(fd, I2CRPTSTART, &cmd);
-		if (error == -1) {
-			err_msg = "ioctl: error sending repeated start "
-			    "condition";
-			goto err1;
-		}
+		if (i2c_do_repeatstart(fd, &cmd))
+			return (i2c_do_stop(fd) | 1);
 	}
 
 	cmd.count = i2c_opt.count;
-	cmd.buf = i2c_buf;
+	cmd.buf = (void*)i2c_buf;
 	cmd.last = 1;
-	error = ioctl(fd, I2CREAD, &cmd);
-	if (error == -1) {
-		err_msg = "ioctl: error while reading";
-		goto err1;
-	}
+	if (i2c_do_read(fd, &cmd))
+		return (i2c_do_stop(fd) | 1);
 
-	error = ioctl(fd, I2CSTOP);
-	if (error == -1) {
-		err_msg = "error sending stop condtion\n";
-		goto err2;
-	}
-
-	return (0);
-
-err1:
-	error = ioctl(fd, I2CSTOP);
-	if (error == -1)
-		fprintf(stderr, "error sending stop condition\n");
-err2:
-	if (err_msg)
-		fprintf(stderr, "%s\n", err_msg);
-
-	return (1);
+	return (i2c_do_stop(fd));
 }
 
 /*
@@ -472,7 +445,7 @@ err2:
  * driver to be handled as a single transfer.
  */
 static int
-i2c_rdwr_transfer(int fd, struct options i2c_opt, char *i2c_buf)
+i2c_rdwr_transfer(int fd, struct options i2c_opt, uint8_t *i2c_buf)
 {
 	struct iic_msg msgs[2], *msgp = msgs;
 	struct iic_rdwr_data xfer;
@@ -515,28 +488,19 @@ i2c_rdwr_transfer(int fd, struct options i2c_opt, char *i2c_buf)
 static int
 access_bus(int fd, struct options i2c_opt)
 {
-	char *i2c_buf;
-	int error, chunk_size = 16, i, ch;
-
-	i2c_buf = malloc(i2c_opt.count);
-	if (i2c_buf == NULL)
-		err(1, "data malloc");
+	uint8_t i2c_buf[i2c_opt.count];
+	int error;
+	unsigned u, chunk_size = 16;
 
 	/*
 	 * For a write, read the data to be written to the chip from stdin.
 	 */
 	if (i2c_opt.dir == 'w') {
 		if (i2c_opt.verbose && !i2c_opt.binary)
-			fprintf(stderr, "Enter %d bytes of data: ",
+			fprintf(stderr, "Enter %u bytes of data: ",
 			    i2c_opt.count);
-		for (i = 0; i < i2c_opt.count; i++) {
-			ch = getchar();
-			if (ch == EOF) {
-				free(i2c_buf);
-				err(1, "not enough data, exiting\n");
-			}
-			i2c_buf[i] = ch;
-		}
+		if (fread(i2c_buf, 1, i2c_opt.count, stdin) != i2c_opt.count)
+			err(1, "not enough data, exiting\n");
 	}
 
 	if (i2c_opt.mode == I2C_MODE_TRANSFER)
@@ -554,17 +518,16 @@ access_bus(int fd, struct options i2c_opt)
 				fprintf(stderr, "\nData %s (hex):\n",
 				    i2c_opt.dir == 'r' ?  "read" : "written");
 
-			for (i = 0; i < i2c_opt.count; i++) {
-				fprintf (stderr, "%02hhx ", i2c_buf[i]);
-				if ((i % chunk_size) == chunk_size - 1)
-					fprintf(stderr, "\n");
+			for (u = 0; u < i2c_opt.count; u++) {
+				printf("%02hhx ", i2c_buf[u]);
+				if ((u % chunk_size) == chunk_size - 1)
+					printf("\n");
 			}
-			if ((i % chunk_size) != 0)
-				fprintf(stderr, "\n");
+			if ((u % chunk_size) != 0)
+				printf("\n");
 		}
 	}
 
-	free(i2c_buf);
 	return (error);
 }
 
@@ -702,7 +665,7 @@ main(int argc, char** argv)
 
 	if (i2c_opt.verbose)
 		fprintf(stderr, "dev: %s, addr: 0x%x, r/w: %c, "
-		    "offset: 0x%02x, width: %s, count: %d\n", dev,
+		    "offset: 0x%02x, width: %s, count: %u\n", dev,
 		    i2c_opt.addr >> 1, i2c_opt.dir, i2c_opt.off,
 		    i2c_opt.width, i2c_opt.count);
 
@@ -715,10 +678,10 @@ main(int argc, char** argv)
 
 	switch (do_what) {
 	case 's':
-		error = scan_bus(dev, fd, i2c_opt.skip);
+		error = scan_bus(dev, fd, i2c_opt.skip, i2c_opt.verbose);
 		break;
 	case 'r':
-		error = reset_bus(dev, fd);
+		error = reset_bus(dev, fd, i2c_opt.verbose);
 		break;
 	case 'a':
 		error = access_bus(fd, i2c_opt);


More information about the dev-commits-src-main mailing list