Get rid of sp_die()

- Add return values to sp_flush_stream(), sp_pass_writen(),
  sp_execute_opbuf(), sp_execute_opbuf_noflush(),
  sp_check_opbuf_usage(), sp_do_read_n().
- Use those return values to propagate errors instead of exiting.
  In some places this has to wait for core API changes (error handling for
  chip_readb, chip_readn, chip_write) hence comments are added instead.

Corresponding to flashrom svn r1719.

Signed-off-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
Acked-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>

programmer.h

@@ -661,7 +661,6 @@ typedef int fdtype;
 void sp_flush_incoming(void);
 fdtype sp_openserport(char *dev, unsigned int baud);
 int serialport_config(fdtype fd, unsigned int baud);
-void __attribute__((noreturn)) sp_die(char *msg);
 extern fdtype sp_fd;
 /* expose serialport_shutdown as it's currently used by buspirate */
 int serialport_shutdown(void *data);

serial.c

@@ -41,12 +41,6 @@
 
 fdtype sp_fd = SER_INV_FD;
 
-void __attribute__((noreturn)) sp_die(char *msg)
-{
-	perror(msg);
-	exit(1);
-}
-
 #ifdef _WIN32
 struct baudentry {
 	DWORD flag;

serprog.c

@@ -237,25 +237,27 @@ static int sp_docommand(uint8_t command, uint32_t parmlen,
 	return 0;
 }
 
-static void sp_flush_stream(void)
+static int sp_flush_stream(void)
 {
 	if (sp_streamed_transmit_ops)
 		do {
 			unsigned char c;
 			if (serialport_read(&c, 1) != 0) {
-				sp_die("Error: cannot read from device (flushing stream)");
+				msg_perr("Error: cannot read from device (flushing stream)");
+				return 1;
 			}
 			if (c == S_NAK) {
 				msg_perr("Error: NAK to a stream buffer operation\n");
-				exit(1);
+				return 1;
 			}
 			if (c != S_ACK) {
 				msg_perr("Error: Invalid reply 0x%02X from device\n", c);
-				exit(1);
+				return 1;
 			}
 		} while (--sp_streamed_transmit_ops);
 	sp_streamed_transmit_ops = 0;
 	sp_streamed_transmit_bytes = 0;
+	return 0;
 }
 
 static int sp_stream_buffer_op(uint8_t cmd, uint32_t parmlen, uint8_t *parms)
@@ -263,17 +265,30 @@ static int sp_stream_buffer_op(uint8_t cmd, uint32_t parmlen, uint8_t * parms)
 	uint8_t *sp;
 	if (sp_automatic_cmdcheck(cmd))
 		return 1;
+
 	sp = malloc(1 + parmlen);
-	if (!sp) sp_die("Error: cannot malloc command buffer");
+	if (!sp) {
+		msg_perr("Error: cannot malloc command buffer\n");
+		return 1;
+	}
 	sp[0] = cmd;
 	memcpy(&(sp[1]), parms, parmlen);
-	if (sp_streamed_transmit_bytes >= (1 + parmlen + sp_device_serbuf_size))
+
-		sp_flush_stream();
+	if (sp_streamed_transmit_bytes >= (1 + parmlen + sp_device_serbuf_size)) {
-	if (serialport_write(sp, 1 + parmlen) != 0)
+		if (sp_flush_stream() != 0) {
-		sp_die("Error: cannot write command");
 			free(sp);
+			return 1;
+		}
+	}
+	if (serialport_write(sp, 1 + parmlen) != 0) {
+		msg_perr("Error: cannot write command\n");
+		free(sp);
+		return 1;
+	}
 	sp_streamed_transmit_ops += 1;
 	sp_streamed_transmit_bytes += 1 + parmlen;
+
+	free(sp);
 	return 0;
 }
 
@@ -656,16 +671,16 @@ int serprog_init(void)
 	return 0;
 }
 
-/* Move an in flashrom buffer existing write-n operation to	*
+/* Move an in flashrom buffer existing write-n operation to the on-device operation buffer. */
- * the on-device operation buffer.				*/
+static int sp_pass_writen(void)
-static void sp_pass_writen(void)
 {
 	unsigned char header[7];
-	msg_pspew(MSGHEADER "Passing write-n bytes=%d addr=0x%x\n",
+	msg_pspew(MSGHEADER "Passing write-n bytes=%d addr=0x%x\n", sp_write_n_bytes, sp_write_n_addr);
-		  sp_write_n_bytes, sp_write_n_addr);
+	if (sp_streamed_transmit_bytes >= (7 + sp_write_n_bytes + sp_device_serbuf_size)) {
-	if (sp_streamed_transmit_bytes >=
+		if (sp_flush_stream() != 0) {
-	    (7 + sp_write_n_bytes + sp_device_serbuf_size))
+			return 1;
-		sp_flush_stream();
+		}
+	}
 	/* In case it's just a single byte send it as a single write. */
 	if (sp_write_n_bytes == 1) {
 		sp_write_n_bytes = 0;
@@ -673,9 +688,10 @@ static void sp_pass_writen(void)
 		header[1] = (sp_write_n_addr >> 8) & 0xFF;
 		header[2] = (sp_write_n_addr >> 16) & 0xFF;
 		header[3] = sp_write_n_buf[0];
-		sp_stream_buffer_op(S_CMD_O_WRITEB, 4, header);
+		if (sp_stream_buffer_op(S_CMD_O_WRITEB, 4, header) != 0)
+			return 1;
 		sp_opbuf_usage += 5;
-		return;
+		return 0;
 	}
 	header[0] = S_CMD_O_WRITEN;
 	header[1] = (sp_write_n_bytes >> 0) & 0xFF;
@@ -684,39 +700,55 @@ static void sp_pass_writen(void)
 	header[4] = (sp_write_n_addr >> 0) & 0xFF;
 	header[5] = (sp_write_n_addr >> 8) & 0xFF;
 	header[6] = (sp_write_n_addr >> 16) & 0xFF;
-	if (serialport_write(header, 7) != 0)
+	if (serialport_write(header, 7) != 0) {
-		sp_die("Error: cannot write write-n command\n");
+		msg_perr(MSGHEADER "Error: cannot write write-n command\n");
-	if (serialport_write(sp_write_n_buf, sp_write_n_bytes) != 0)
+		return 1;
-		sp_die("Error: cannot write write-n data");
+	}
+	if (serialport_write(sp_write_n_buf, sp_write_n_bytes) != 0) {
+		msg_perr(MSGHEADER "Error: cannot write write-n data");
+		return 1;
+	}
 	sp_streamed_transmit_bytes += 7 + sp_write_n_bytes;
 	sp_streamed_transmit_ops += 1;
 	sp_opbuf_usage += 7 + sp_write_n_bytes;
 	sp_write_n_bytes = 0;
 	sp_prev_was_write = 0;
+	return 0;
 }
 
-static void sp_execute_opbuf_noflush(void)
+static int sp_execute_opbuf_noflush(void)
 {
-	if ((sp_max_write_n) && (sp_write_n_bytes))
+	if ((sp_max_write_n) && (sp_write_n_bytes)) {
-		sp_pass_writen();
+		if (sp_pass_writen() != 0) {
-	sp_stream_buffer_op(S_CMD_O_EXEC, 0, NULL);
+			msg_perr("Error: could not transfer write buffer\n");
-	msg_pspew(MSGHEADER "Executed operation buffer of %d bytes\n",
+			return 1;
-		     sp_opbuf_usage);
+		}
+	}
+	if (sp_stream_buffer_op(S_CMD_O_EXEC, 0, NULL) != 0) {
+		msg_perr("Error: could not execute command buffer\n");
+		return 1;
+	}
+	msg_pspew(MSGHEADER "Executed operation buffer of %d bytes\n", sp_opbuf_usage);
 	sp_opbuf_usage = 0;
 	sp_prev_was_write = 0;
-	return;
+	return 0;
 }
 
-static void sp_execute_opbuf(void)
+static int sp_execute_opbuf(void)
 {
-	sp_execute_opbuf_noflush();
+	if (sp_execute_opbuf_noflush() != 0)
-	sp_flush_stream();
+		return 1;
+	if (sp_flush_stream() != 0)
+		return 1;
+
+	return 0;
 }
 
 static int serprog_shutdown(void *data)
 {
 	if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes))
-		sp_execute_opbuf();
+	if (sp_execute_opbuf() != 0)
+		msg_pwarn("Could not flush command buffer.\n");
 	if (sp_check_commandavail(S_CMD_S_PIN_STATE)) {
 		uint8_t dis = 0;
 		if (sp_docommand(S_CMD_S_PIN_STATE, 1, &dis, 0, NULL) == 0)
@@ -731,14 +763,15 @@ static int serprog_shutdown(void *data)
 	return 0;
 }
 
-static void sp_check_opbuf_usage(int bytes_to_be_added)
+static int sp_check_opbuf_usage(int bytes_to_be_added)
 {
 	if (sp_device_opbuf_size <= (sp_opbuf_usage + bytes_to_be_added)) {
-		sp_execute_opbuf();
+		/* If this happens in the middle of a page load the page load will probably fail. */
-		/* If this happens in the mid of an page load the page load *
+		msg_pwarn(MSGHEADER "Warning: executed operation buffer due to size reasons\n");
-		 * will probably fail.					    */
+		if (sp_execute_opbuf() != 0)
-		msg_pdbg(MSGHEADER "Warning: executed operation buffer due to size reasons\n");
+			return 1;
 	}
+	return 0;
 }
 
 static void serprog_chip_writeb(const struct flashctx *flash, uint8_t val,
@@ -768,7 +801,7 @@ static void serprog_chip_writeb(const struct flashctx *flash, uint8_t val,
 		writeb_parm[1] = (addr >> 8) & 0xFF;
 		writeb_parm[2] = (addr >> 16) & 0xFF;
 		writeb_parm[3] = val;
-		sp_stream_buffer_op(S_CMD_O_WRITEB, 4, writeb_parm);
+		sp_stream_buffer_op(S_CMD_O_WRITEB, 4, writeb_parm); // FIXME: return error
 		sp_opbuf_usage += 5;
 	}
 }
@@ -785,16 +818,16 @@ static uint8_t serprog_chip_readb(const struct flashctx *flash,
 	buf[0] = ((addr >> 0) & 0xFF);
 	buf[1] = ((addr >> 8) & 0xFF);
 	buf[2] = ((addr >> 16) & 0xFF);
-	sp_stream_buffer_op(S_CMD_R_BYTE, 3, buf);
+	sp_stream_buffer_op(S_CMD_R_BYTE, 3, buf); // FIXME: return error
-	sp_flush_stream();
+	sp_flush_stream(); // FIXME: return error
 	if (serialport_read(&c, 1) != 0)
-		sp_die("readb byteread");
+		msg_perr(MSGHEADER "readb byteread");  // FIXME: return error
 	msg_pspew("%s addr=0x%" PRIxPTR " returning 0x%02X\n", __func__, addr, c);
 	return c;
 }
 
 /* Local version that really does the job, doesn't care of max_read_n. */
-static void sp_do_read_n(uint8_t * buf, const chipaddr addr, size_t len)
+static int sp_do_read_n(uint8_t * buf, const chipaddr addr, size_t len)
 {
 	unsigned char sbuf[6];
 	msg_pspew("%s: addr=0x%" PRIxPTR " len=%zu\n", __func__, addr, len);
@@ -808,10 +841,13 @@ static void sp_do_read_n(uint8_t * buf, const chipaddr addr, size_t len)
 	sbuf[4] = ((len >> 8) & 0xFF);
 	sbuf[5] = ((len >> 16) & 0xFF);
 	sp_stream_buffer_op(S_CMD_R_NBYTES, 6, sbuf);
-	sp_flush_stream();
+	if (sp_flush_stream() != 0)
-	if (serialport_read(buf, len) != 0)
+		return 1;
-		sp_die("Error: cannot read read-n data");
+	if (serialport_read(buf, len) != 0) {
-	return;
+		msg_perr(MSGHEADER "Error: cannot read read-n data");
+		return 1;
+	}
+	return 0;
 }
 
 /* The externally called version that makes sure that max_read_n is obeyed. */
@@ -821,12 +857,12 @@ static void serprog_chip_readn(const struct flashctx *flash, uint8_t * buf,
 	size_t lenm = len;
 	chipaddr addrm = addr;
 	while ((sp_max_read_n != 0) && (lenm > sp_max_read_n)) {
-		sp_do_read_n(&(buf[addrm-addr]), addrm, sp_max_read_n);
+		sp_do_read_n(&(buf[addrm-addr]), addrm, sp_max_read_n); // FIXME: return error
 		addrm += sp_max_read_n;
 		lenm -= sp_max_read_n;
 	}
 	if (lenm)
-		sp_do_read_n(&(buf[addrm-addr]), addrm, lenm);
+		sp_do_read_n(&(buf[addrm-addr]), addrm, lenm); // FIXME: return error
 }
 
 void serprog_delay(int usecs)
@@ -858,11 +894,18 @@ static int serprog_spi_send_command(struct flashctx *flash,
 	unsigned char *parmbuf;
 	int ret;
 	msg_pspew("%s, writecnt=%i, readcnt=%i\n", __func__, writecnt, readcnt);
-	if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes))
+	if ((sp_opbuf_usage) || (sp_max_write_n && sp_write_n_bytes)) {
-		sp_execute_opbuf();
+		if (sp_execute_opbuf() != 0) {
+			msg_perr("Error: could not execute command buffer before sending SPI commands.\n");
+			return 1;
+		}
+	}
+
 	parmbuf = malloc(writecnt + 6);
-	if (!parmbuf)
+	if (!parmbuf) {
-		sp_die("Error: cannot malloc SPI send param buffer");
+		msg_perr("Error: could not allocate SPI send param buffer.\n");
+		return 1;
+	}
 	parmbuf[0] = (writecnt >> 0) & 0xFF;
 	parmbuf[1] = (writecnt >> 8) & 0xFF;
 	parmbuf[2] = (writecnt >> 16) & 0xFF;