[flashrom] [PATCH] remove exit calls from sp_sync_read_timeout and sp_synchronize

Niklas Söderlund niso at kth.se
Fri Jun 1 22:39:58 CEST 2012


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

The addition of a return code to signal a failure from the read syscall
in sp_sync_read_timeout is a bit of a stretch and might look strange.
But the only caller is in the loop in sp_synchronize and any sequential
call to read will result in the same error and end result. But please
take a extra look on this to make sure we don't miss any corner case.

The alternative would be to differentiate between a read failure over
fast detection of a EAGAIN. The end result would in my opinion only make
the code harder to read. The cost is that upon read failure the user
might be spammed up to 10 times with the same error message before
flashrom dies.

Signed-off-by: Niklas Söderlund <niso at kth.se>
---
 serprog.c |   35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/serprog.c b/serprog.c
index 02996ea..f03d332 100644
--- a/serprog.c
+++ b/serprog.c
@@ -145,8 +145,10 @@ static int sp_sync_read_timeout(int loops)
 		rv = read(sp_fd, &c, 1);
 		if (rv == 1)
 			return c;
-		if ((rv == -1) && (errno != EAGAIN))
-			sp_die("read");
+		if ((rv == -1) && (errno != EAGAIN)) {
+			msg_perr("read: %s\n", strerror(errno));
+			return -1;
+		}
 		usleep(10 * 1000);	/* 10ms units */
 	}
 	return -1;
@@ -158,7 +160,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);
@@ -169,8 +171,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));
+		return 1;
+	}
 	/* A second should be enough to get all the answers to the buffer */
 	usleep(1000 * 1000);
 	sp_flush_incoming();
@@ -181,9 +185,11 @@ static void sp_synchronize(void)
 	 * attempt, ~1s if immediate success.                                 */
 	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");
+		int c = S_CMD_SYNCNOP;
+		if (write(sp_fd, &c, 1) != 1) {
+			msg_perr("sync write: %s\n", strerror(errno));
+			return 1;
+		}
 		msg_pdbg(".");
 		fflush(stdout);
 		for (n = 0; n < 10; n++) {
@@ -194,8 +200,10 @@ static void sp_synchronize(void)
 			if (c != S_ACK)
 				continue;
 			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));
+				return 1;
+			}
 			c = sp_sync_read_timeout(50);
 			if (c != S_NAK)
 				break;	/* fail */
@@ -206,12 +214,12 @@ static void sp_synchronize(void)
 			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);
+	return 1;
 }
 
 static int sp_check_commandavail(uint8_t command)
@@ -432,7 +440,8 @@ int serprog_init(void)
 
 	sp_check_avail_automatic = 0;
 
-	sp_synchronize();
+	if (sp_synchronize())
+		return 1;
 
 	msg_pdbg(MSGHEADER "Synchronized\n");
 
-- 
1.7.10.3





More information about the flashrom mailing list