[flashrom] [PATCH] Bus Pirate buffer management revamp

[Carl-Daniel Hailfinger]

Use separate functions for managing the Bus Pirate command/data buffer.
Open-coding the buffer management was the first step, now the goal is to
make it readable.
This is the buffer management part of
"Re: [flashrom] [PATCH] Make Bus Pirate init more robust, speed up flashing"

The buffer management of the Bus Pirate driver has been revamped
to use grow-only buffers with a reasonable initial default size
so realloc() will not have to be called in normal operation.
A side effect is the ability to switch to a static buffer without
major hassle.
Handle OOM gracefully.


Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: flashrom-buspirate_buffermanagement/flash.h
===================================================================
--- flashrom-buspirate_buffermanagement/flash.h	(Revision 1184)
+++ flashrom-buspirate_buffermanagement/flash.h	(Arbeitskopie)
@@ -35,6 +35,9 @@
 
 #define ERROR_PTR ((void*)-1)
 
+/* Error codes */
+#define OOM_ERROR	-100
+
 typedef unsigned long chipaddr;
 
 int register_shutdown(void (*function) (void *data), void *data);
Index: flashrom-buspirate_buffermanagement/buspirate_spi.c
===================================================================
--- flashrom-buspirate_buffermanagement/buspirate_spi.c	(Revision 1184)
+++ flashrom-buspirate_buffermanagement/buspirate_spi.c	(Arbeitskopie)
@@ -46,6 +46,55 @@
 #define sp_flush_incoming(...) 0
 #endif
 
+static unsigned char *bp_commbuf = NULL;
+static int bp_commbufsize = 0;
+
+static int buspirate_commbuf_resize(int bufsize)
+{
+	unsigned char *tmpbuf;
+
+	/* Never shrink. realloc() calls are expensive. */
+	if (bufsize <= bp_commbufsize)
+		return 0;
+
+	tmpbuf = realloc(bp_commbuf, bufsize);
+	if (!tmpbuf) {
+		/* This is debatable. Do we really want to free the existing
+		 * buffer or do we want to keep it around, especially if memory
+		 * is already tight?
+		 */
+		free(bp_commbuf);
+		bp_commbuf = NULL;
+		bp_commbufsize = 0;
+		msg_perr("Out of memory!\n");
+		return OOM_ERROR;
+	}
+
+	bp_commbuf = tmpbuf;
+	bp_commbufsize = bufsize;
+	return 0;
+}
+
+static int buspirate_commbuf_init(int bufsize)
+{
+	bp_commbuf = malloc(bufsize);
+	if (!bp_commbuf) {
+		bp_commbufsize = 0;
+		msg_perr("Out of memory!\n");
+		return OOM_ERROR;
+	}
+
+	bp_commbufsize = bufsize;
+	return 0;
+}
+
+static void buspirate_commbuf_shutdown(void)
+{
+	free(bp_commbuf);
+	bp_commbuf = NULL;
+	bp_commbufsize = 0;
+}
+
 static int buspirate_sendrecv(unsigned char *buf, unsigned int writecnt, unsigned int readcnt)
 {
 	int i, ret = 0;
@@ -231,6 +280,10 @@
 		return 1;
 	}
 
+	/* Sensible default buffer size. */
+	if (buspirate_commbuf_init(16 + 3))
+		return OOM_ERROR;
+
 	buses_supported = CHIP_BUSTYPE_SPI;
 	spi_controller = SPI_CONTROLLER_BUSPIRATE;
 
@@ -267,6 +320,9 @@
 	ret = serialport_shutdown();
 	if (ret)
 		return ret;
+
+	buspirate_commbuf_shutdown();
+
 	msg_pdbg("Bus Pirate shutdown completed.\n");
 
 	return 0;
@@ -275,55 +331,51 @@
 int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt,
 		const unsigned char *writearr, unsigned char *readarr)
 {
-	static unsigned char *buf = NULL;
 	int i = 0, ret = 0;
 
 	if (writecnt > 16 || readcnt > 16 || (readcnt + writecnt) > 16)
 		return SPI_INVALID_LENGTH;
 
 	/* 3 bytes extra for CS#, len, CS#. */
-	buf = realloc(buf, writecnt + readcnt + 3);
-	if (!buf) {
-		msg_perr("Out of memory!\n");
-		exit(1); // -1
-	}
+	if (buspirate_commbuf_resize(writecnt + readcnt + 3))
+		return OOM_ERROR;
 
 	/* Assert CS# */
-	buf[i++] = 0x02;
+	bp_commbuf[i++] = 0x02;
 
-	buf[i++] = 0x10 | (writecnt + readcnt - 1);
-	memcpy(buf + i, writearr, writecnt);
+	bp_commbuf[i++] = 0x10 | (writecnt + readcnt - 1);
+	memcpy(bp_commbuf + i, writearr, writecnt);
 	i += writecnt;
-	memset(buf + i, 0, readcnt);
+	memset(bp_commbuf + i, 0, readcnt);
 
 	i += readcnt;
 	/* De-assert CS# */
-	buf[i++] = 0x03;
+	bp_commbuf[i++] = 0x03;
 
-	ret = buspirate_sendrecv(buf, i, i);
+	ret = buspirate_sendrecv(bp_commbuf, i, i);
 
 	if (ret) {
 		msg_perr("Bus Pirate communication error!\n");
 		return SPI_GENERIC_ERROR;
 	}
 
-	if (buf[0] != 0x01) {
+	if (bp_commbuf[0] != 0x01) {
 		msg_perr("Protocol error while lowering CS#!\n");
 		return SPI_GENERIC_ERROR;
 	}
 
-	if (buf[1] != 0x01) {
+	if (bp_commbuf[1] != 0x01) {
 		msg_perr("Protocol error while reading/writing SPI!\n");
 		return SPI_GENERIC_ERROR;
 	}
 
-	if (buf[i - 1] != 0x01) {
+	if (bp_commbuf[i - 1] != 0x01) {
 		msg_perr("Protocol error while raising CS#!\n");
 		return SPI_GENERIC_ERROR;
 	}
 
 	/* Skip CS#, length, writearr. */
-	memcpy(readarr, buf + 2 + writecnt, readcnt);
+	memcpy(readarr, bp_commbuf + 2 + writecnt, readcnt);
 
 	return ret;
 }


-- 
http://www.hailfinger.org/