Patchwork ft2232_spi: Improve error handling, remove exit() calls

login
register
about
Submitter Uwe Hermann
Date 2011-07-20 22:37:39
Message ID <20110720223739.GC4802@greenwood>
Download mbox | patch
Permalink /patch/3283/
State Accepted
Commit r1377
Headers show

Comments

Uwe Hermann - 2011-07-20 22:37:39
See patch.

This is just a first step, there is certainly room for further improving
the error handling (error #defines such as FL_ERR_MALLOC, consistent and
documented return codes, and more).


Uwe.
Tadas Slotkus - 2011-07-20 23:50:36
Hi,
>                 buf = realloc(buf, bufsize);
>                 if (!buf) {
>                         msg_perr("Out of memory!\n");
> -                       exit(1);
> +                       /* TODO: What to do with buf? */
> +                       return SPI_GENERIC_ERROR;
>                 }
Not looked very deeply, but looks like this is not freed even when in
normal execution(until program termination). Checkings would add ugly
lines. I do believe that we won't end up in memleaks, so if I have a
right, then:
Acked-by: Tadas Slotkus <devtadas@gmail.com>

By the way, great job in wiki and flashrom GUI :)

Thanks,
Tadas
Uwe Hermann - 2011-07-21 18:25:11
On Thu, Jul 21, 2011 at 02:50:36AM +0300, Tadas Slotkus wrote:
> Hi,
> >                 buf = realloc(buf, bufsize);
> >                 if (!buf) {
> >                         msg_perr("Out of memory!\n");
> > -                       exit(1);
> > +                       /* TODO: What to do with buf? */
> > +                       return SPI_GENERIC_ERROR;
> >                 }
> Not looked very deeply, but looks like this is not freed even when in
> normal execution(until program termination). Checkings would add ugly
> lines. I do believe that we won't end up in memleaks, so if I have a

You're probably right, yes, I left the TODO in there for now, until
we're sure.


> right, then:
> Acked-by: Tadas Slotkus <devtadas@gmail.com>

Thanks, r1377.


Uwe.

Patch

ft2232_spi: Improve error handling, remove exit() calls.

In order to make the ft2232_spi code more usable in libflashrom (e.g. from
frontends/GUIs) there must not be any exit() calls in the code, as that
would also terminate the frontend. Thus, replace all exit() calls with
proper error handling code by returning a _unique_ negative error number,
so that the frontend (and/or user/developer) can also know a bit more
exactly _which_ error occured, not only _that_ an error occured.

Also, call ftdi_usb_close() before returning due to errors.

Signed-off-by: Uwe Hermann <uwe@hermann-uwe.de>

Index: ft2232_spi.c
===================================================================
--- ft2232_spi.c	(Revision 1376)
+++ ft2232_spi.c	(Arbeitskopie)
@@ -150,9 +150,10 @@ 
 	.write_256 = default_spi_write_256,
 };
 
+/* Returns 0 upon success, a negative number upon errors. */
 int ft2232_spi_init(void)
 {
-	int f;
+	int f, ret = 0;
 	struct ftdi_context *ftdic = &ftdic_context;
 	unsigned char buf[512];
 	int ft2232_vid = FTDI_VID;
@@ -195,7 +196,7 @@ 
 		} else {
 			msg_perr("Error: Invalid device type specified.\n");
 			free(arg);
-			return 1;
+			return -1;
 		}
 	}
 	free(arg);
@@ -211,7 +212,7 @@ 
 		default:
 			msg_perr("Error: Invalid port/interface specified.\n");
 			free(arg);
-			return 1;
+			return -2;
 		}
 	}
 	free(arg);
@@ -223,7 +224,7 @@ 
 
 	if (ftdi_init(ftdic) < 0) {
 		msg_perr("ftdi_init failed\n");
-		return EXIT_FAILURE; // TODO
+		return -3;
 	}
 
 	f = ftdi_usb_open(ftdic, ft2232_vid, ft2232_type);
@@ -231,7 +232,7 @@ 
 	if (f < 0 && f != -5) {
 		msg_perr("Unable to open FTDI device: %d (%s)\n", f,
 				ftdi_get_error_string(ftdic));
-		exit(-1); // TODO
+		return -4;
 	}
 
 	if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H) {
@@ -264,8 +265,10 @@ 
 	if (clock_5x) {
 		msg_pdbg("Disable divide-by-5 front stage\n");
 		buf[0] = 0x8a;		/* Disable divide-by-5. */
-		if (send_buf(ftdic, buf, 1))
-			return -1;
+		if (send_buf(ftdic, buf, 1)) {
+			ret = -5;
+			goto ftdi_err;
+		}
 		mpsse_clk = 60.0;
 	} else {
 		mpsse_clk = 12.0;
@@ -276,8 +279,10 @@ 
 	/* valueL/valueH are (desired_divisor - 1) */
 	buf[1] = (DIVIDE_BY - 1) & 0xff;
 	buf[2] = ((DIVIDE_BY - 1) >> 8) & 0xff;
-	if (send_buf(ftdic, buf, 3))
-		return -1;
+	if (send_buf(ftdic, buf, 3)) {
+		ret = -6;
+		goto ftdi_err;
+	}
 
 	msg_pdbg("MPSSE clock: %f MHz divisor: %d "
 		 "SPI clock: %f MHz\n",
@@ -287,23 +292,36 @@ 
 	/* Disconnect TDI/DO to TDO/DI for loopback. */
 	msg_pdbg("No loopback of TDI/DO TDO/DI\n");
 	buf[0] = 0x85;
-	if (send_buf(ftdic, buf, 1))
-		return -1;
+	if (send_buf(ftdic, buf, 1)) {
+		ret = -7;
+		goto ftdi_err;
+	}
 
 	msg_pdbg("Set data bits\n");
 	buf[0] = SET_BITS_LOW;
 	buf[1] = cs_bits;
 	buf[2] = pindir;
-	if (send_buf(ftdic, buf, 3))
-		return -1;
+	if (send_buf(ftdic, buf, 3)) {
+		ret = -8;
+		goto ftdi_err;
+	}
 
 	// msg_pdbg("\nft2232 chosen\n");
 
 	register_spi_programmer(&spi_programmer_ft2232);
 
 	return 0;
+
+ftdi_err:
+	if ((f = ftdi_usb_close(ftdic)) < 0) {
+		msg_perr("Unable to close FTDI device: %d (%s)\n", f,
+		         ftdi_get_error_string(ftdic));
+	}
+
+	return ret;
 }
 
+/* Returns 0 upon success, a negative number upon errors. */
 static int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt,
 		const unsigned char *writearr, unsigned char *readarr)
 {
@@ -324,7 +342,8 @@ 
 		buf = realloc(buf, bufsize);
 		if (!buf) {
 			msg_perr("Out of memory!\n");
-			exit(1);
+			/* TODO: What to do with buf? */
+			return SPI_GENERIC_ERROR;
 		}
 		oldbufsize = bufsize;
 	}