[flashrom] [PATCH] Remove exit calls from sp_sync_read_timeout and sp_synchronize.

[Stefan Tauner]

Add return values to sp_synchronize so we can signal a failure to the
only upstream caller (serprog_init), which is prepared to propagate a failure.

sp_sync_read_timeout was harder to fix because it already used a return
value, but we needed to distinguish two different failure modes. This
solution distinguishes them by the sign of the return values, which maintains
readability as much as possible.

Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
---
 serial.c  |    1 +
 serprog.c |   76 ++++++++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/serial.c b/serial.c
index 05c7d04..7e47dcc 100644
--- a/serial.c
+++ b/serial.c
@@ -239,6 +239,7 @@ void sp_flush_incoming(void)
 #ifdef _WIN32
 	PurgeComm(sp_fd, PURGE_RXCLEAR);
 #else
+	/* FIXME: error handling */
 	tcflush(sp_fd, TCIFLUSH);
 #endif
 	return;
diff --git a/serprog.c b/serprog.c
index dd86fd3..957e38f 100644
--- a/serprog.c
+++ b/serprog.c
@@ -138,20 +138,24 @@ static int sp_opensocket(char *ip, unsigned int port)
 	return sock;
 }
 
-static int sp_sync_read_timeout(int loops)
+/* Returns 0 on success and places the character read into the location pointed to by c.
+ * Returns positive values on temporary errors and negative ones on permanent errors.
+ * An iteration of one loop takes up to 1ms. */
+static int sp_sync_read_timeout(unsigned int loops, unsigned char *c)
 {
 	int i;
-	unsigned char c;
 	for (i = 0; i < loops; i++) {
 		ssize_t rv;
-		rv = read(sp_fd, &c, 1);
+		rv = read(sp_fd, c, 1);
 		if (rv == 1)
-			return c;
-		if ((rv == -1) && (errno != EAGAIN))
-			sp_die("read");
-		usleep(10 * 1000);	/* 10ms units */
+			return 0;
+		if ((rv == -1) && (errno != EAGAIN)) {
+			msg_perr("read: %s\n", strerror(errno));
+			return -1;
+		}
+		usleep(1000);	/* 1ms units */
 	}
-	return -1;
+	return 1;
 }
 
 /* Synchronize: a bit tricky algorithm that tries to (and in my tests has *
@@ -160,7 +164,7 @@ static int sp_sync_read_timeout(int loops)
  * blocking read - TODO: add an alarm() timer for the rest of the app on  *
  * serial operations, though not such a big issue as the first thing to   *
  * do is synchronize (eg. check that device is alive).			  */
-static void sp_synchronize(void)
+static int sp_synchronize(void)
 {
 	int i;
 	int flags = fcntl(sp_fd, F_GETFL);
@@ -171,8 +175,10 @@ static void sp_synchronize(void)
 	 * the device serial parser to get to a sane state, unless if it   *
 	 * is waiting for a real long write-n.                             */
 	memset(buf, S_CMD_NOP, 8);
-	if (write(sp_fd, buf, 8) != 8)
-		sp_die("flush write");
+	if (write(sp_fd, buf, 8) != 8) {
+		msg_perr("flush write: %s\n", strerror(errno));
+		goto err_out;
+	}
 	/* A second should be enough to get all the answers to the buffer */
 	usleep(1000 * 1000);
 	sp_flush_incoming();
@@ -184,36 +190,48 @@ static void sp_synchronize(void)
 	for (i = 0; i < 8; i++) {
 		int n;
 		unsigned char c = S_CMD_SYNCNOP;
-		if (write(sp_fd, &c, 1) != 1)
-			sp_die("sync write");
+		if (write(sp_fd, &c, 1) != 1) {
+			msg_perr("sync write: %s\n", strerror(errno));
+			goto err_out;
+		}
 		msg_pdbg(".");
 		fflush(stdout);
 		for (n = 0; n < 10; n++) {
-			c = sp_sync_read_timeout(5);	/* wait up to 50ms */
-			if (c != S_NAK)
+			unsigned char ret = sp_sync_read_timeout(50, &c);
+			if (ret < 0)
+				goto err_out;
+			if (ret > 0 || c != S_NAK)
 				continue;
-			c = sp_sync_read_timeout(2);
-			if (c != S_ACK)
+			ret = sp_sync_read_timeout(20, &c);
+			if (ret < 0)
+				goto err_out;
+			if (ret > 0 || c != S_ACK)
 				continue;
 			c = S_CMD_SYNCNOP;
-			if (write(sp_fd, &c, 1) != 1)
-				sp_die("sync write");
-			c = sp_sync_read_timeout(50);
-			if (c != S_NAK)
+			if (write(sp_fd, &c, 1) != 1) {
+				msg_perr("sync write: %s\n", strerror(errno));
+				return 1;
+			}
+			ret = sp_sync_read_timeout(500, &c);
+			if (ret < 0)
+				goto err_out;
+			if (ret > 0 || c != S_NAK)
 				break;	/* fail */
-			c = sp_sync_read_timeout(10);
+			ret = sp_sync_read_timeout(100, &c);
+			if (ret > 0 || ret < 0)
+				goto err_out;
 			if (c != S_ACK)
 				break;	/* fail */
 			/* Ok, synchronized; back to blocking reads and return. */
 			flags &= ~O_NONBLOCK;
 			fcntl(sp_fd, F_SETFL, flags);
 			msg_pdbg("\n");
-			return;
+			return 0;
 		}
 	}
-	msg_perr("Error: cannot synchronize protocol "
-		"- check communications and reset device?\n");
-	exit(1);
+err_out:
+	msg_perr("Error: cannot synchronize protocol - check communications and reset device?\n");
+	return 1;
 }
 
 static int sp_check_commandavail(uint8_t command)
@@ -443,7 +461,8 @@ int serprog_init(void)
 
 	sp_check_avail_automatic = 0;
 
-	sp_synchronize();
+	if (sp_synchronize())
+		return 1;
 
 	msg_pdbg(MSGHEADER "Synchronized\n");
 
@@ -743,7 +762,8 @@ static int serprog_shutdown(void *data)
 	msg_pspew("%s\n", __func__);
 	if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes))
 		sp_execute_opbuf();
-	close(sp_fd);
+	/* FIXME: fix sockets on windows(?), especially closing */
+	serialport_shutdown(&sp_fd);
 	if (sp_max_write_n)
 		free(sp_write_n_buf);
 	return 0;
-- 
Kind regards, Stefan Tauner