[U-Boot] [PATCH 2/6] Separate flash read and write operations

Guennadi Liakhovetski lg at denx.de
Wed Aug 27 17:52:41 CEST 2008


The flash_io function was used for both read and write operations, whereby
very little code was shared between the two modes. By breaking this function
we simplify the code and save one level of identation.

Signed-off-by: Guennadi Liakhovetski <lg at denx.de>
---
 tools/env/fw_env.c |  280 ++++++++++++++++++++++++++++------------------------
 1 files changed, 149 insertions(+), 131 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 35783c5..6f7fdb2 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -173,7 +173,8 @@ static char default_environment[] = {
 	"\0"			/* Termimate struct environment data with 2 NULs */
 };
 
-static int flash_io (int mode);
+static int flash_write (void);
+static int flash_read (void);
 static char *envmatch (char * s1, char * s2);
 static int env_init (void);
 static int parse_config (void);
@@ -404,7 +405,7 @@ int fw_setenv (int argc, char *argv[])
 					       ENV_SIZE);
 
 	/* write environment back to flash */
-	if (flash_io (O_RDWR)) {
+	if (flash_write ()) {
 		fprintf (stderr, "Error: can't write fw_env to flash\n");
 		return (-1);
 	}
@@ -412,165 +413,182 @@ int fw_setenv (int argc, char *argv[])
 	return (0);
 }
 
-static int flash_io (int mode)
+static int flash_write (void)
 {
 	int fd, fdr, rc, otherdev, resid;
 	erase_info_t erase;
 	char *data = NULL;
 
-	if ((fd = open (DEVNAME (curdev), mode)) < 0) {
+	if ((fd = open (DEVNAME (curdev), O_RDWR)) < 0) {
 		fprintf (stderr,
-			"Can't open %s: %s\n",
-			DEVNAME (curdev), strerror (errno));
+			 "Can't open %s: %s\n",
+			 DEVNAME (curdev), strerror (errno));
 		return (-1);
 	}
 
-	if (mode == O_RDWR) {
-		if (HaveRedundEnv) {
-			/* switch to next partition for writing */
-			otherdev = !curdev;
-			if ((fdr = open (DEVNAME (otherdev), mode)) < 0) {
-				fprintf (stderr,
-					"Can't open %s: %s\n",
-					DEVNAME (otherdev),
-					strerror (errno));
-				return (-1);
-			}
-		} else {
-			otherdev = curdev;
-			fdr = fd;
-		}
-		printf ("Unlocking flash...\n");
-		erase.length = DEVESIZE (otherdev);
-		erase.start = DEVOFFSET (otherdev);
-		ioctl (fdr, MEMUNLOCK, &erase);
-
-		if (HaveRedundEnv) {
-			erase.length = DEVESIZE (curdev);
-			erase.start = DEVOFFSET (curdev);
-			ioctl (fd, MEMUNLOCK, &erase);
-			ENV_FLAGS(environment) = active_flag;
+	if (HaveRedundEnv) {
+		/* switch to next partition for writing */
+		otherdev = !curdev;
+		if ((fdr = open (DEVNAME (otherdev), O_RDWR)) < 0) {
+			fprintf (stderr,
+				 "Can't open %s: %s\n",
+				 DEVNAME (otherdev),
+				 strerror (errno));
+			return (-1);
 		}
+	} else {
+		otherdev = curdev;
+		fdr = fd;
+	}
+	printf ("Unlocking flash...\n");
+	erase.length = DEVESIZE (otherdev);
+	erase.start = DEVOFFSET (otherdev);
+	ioctl (fdr, MEMUNLOCK, &erase);
+
+	if (HaveRedundEnv) {
+		erase.length = DEVESIZE (curdev);
+		erase.start = DEVOFFSET (curdev);
+		ioctl (fd, MEMUNLOCK, &erase);
+		ENV_FLAGS(environment) = active_flag;
+	}
 
-		printf ("Done\n");
-		resid = DEVESIZE (otherdev) - CFG_ENV_SIZE;
-		if (resid) {
-			if ((data = malloc (resid)) == NULL) {
-				fprintf (stderr,
-					"Cannot malloc %d bytes: %s\n",
-					resid,
-					strerror (errno));
-				return (-1);
-			}
-			if (lseek (fdr, DEVOFFSET (otherdev) + CFG_ENV_SIZE, SEEK_SET)
-				== -1) {
-				fprintf (stderr, "seek error on %s: %s\n",
-					DEVNAME (otherdev),
-					strerror (errno));
-				return (-1);
-			}
-			if ((rc = read (fdr, data, resid)) != resid) {
-				fprintf (stderr,
-					"read error on %s: %s\n",
-					DEVNAME (otherdev),
-					strerror (errno));
-				return (-1);
-			}
+	printf ("Done\n");
+	resid = DEVESIZE (otherdev) - CFG_ENV_SIZE;
+	if (resid) {
+		if ((data = malloc (resid)) == NULL) {
+			fprintf (stderr,
+				 "Cannot malloc %d bytes: %s\n",
+				 resid,
+				 strerror (errno));
+			return (-1);
 		}
-
-		printf ("Erasing old environment...\n");
-
-		erase.length = DEVESIZE (otherdev);
-		erase.start = DEVOFFSET (otherdev);
-		if (ioctl (fdr, MEMERASE, &erase) != 0) {
-			fprintf (stderr, "MTD erase error on %s: %s\n",
-				DEVNAME (otherdev),
-				strerror (errno));
+		if (lseek (fdr, DEVOFFSET (otherdev) + CFG_ENV_SIZE, SEEK_SET)
+		    == -1) {
+			fprintf (stderr, "seek error on %s: %s\n",
+				 DEVNAME (otherdev),
+				 strerror (errno));
 			return (-1);
 		}
-
-		printf ("Done\n");
-
-		printf ("Writing environment to %s...\n", DEVNAME (otherdev));
-		if (lseek (fdr, DEVOFFSET (otherdev), SEEK_SET) == -1) {
+		if ((rc = read (fdr, data, resid)) != resid) {
 			fprintf (stderr,
-				"seek error on %s: %s\n",
-				DEVNAME (otherdev), strerror (errno));
+				 "read error on %s: %s\n",
+				 DEVNAME (otherdev),
+				 strerror (errno));
 			return (-1);
 		}
+	}
+
+	printf ("Erasing old environment...\n");
+
+	erase.length = DEVESIZE (otherdev);
+	erase.start = DEVOFFSET (otherdev);
+	if (ioctl (fdr, MEMERASE, &erase) != 0) {
+		fprintf (stderr, "MTD erase error on %s: %s\n",
+			 DEVNAME (otherdev),
+			 strerror (errno));
+		return (-1);
+	}
+
+	printf ("Done\n");
+
+	printf ("Writing environment to %s...\n", DEVNAME (otherdev));
+	if (lseek (fdr, DEVOFFSET (otherdev), SEEK_SET) == -1) {
+		fprintf (stderr,
+			 "seek error on %s: %s\n",
+			 DEVNAME (otherdev), strerror (errno));
+		return (-1);
+	}
+
+	if (write (fdr, environment.image, CFG_ENV_SIZE) != CFG_ENV_SIZE) {
+		fprintf (stderr,
+			 "Write error on %s: %s\n",
+			 DEVNAME (otherdev), strerror (errno));
+		return (-1);
+	}
 
-		if (write (fdr, environment.image, CFG_ENV_SIZE) !=
-		    CFG_ENV_SIZE) {
+	if (resid) {
+		if (write (fdr, data, resid) != resid) {
 			fprintf (stderr,
-				"Write error on %s: %s\n",
-				DEVNAME (otherdev), strerror (errno));
+				 "write error on %s: %s\n",
+				 DEVNAME (curdev), strerror (errno));
 			return (-1);
 		}
-
-		if (resid) {
-			if (write (fdr, data, resid) != resid) {
-				fprintf (stderr,
-					"write error on %s: %s\n",
-					DEVNAME (curdev), strerror (errno));
-				return (-1);
-			}
-			free (data);
-		}
-		if (HaveRedundEnv) {
-			/* change flag on current active env partition */
-			if (lseek (fd, DEVOFFSET (curdev) + sizeof (ulong), SEEK_SET)
-				== -1) {
-				fprintf (stderr, "seek error on %s: %s\n",
-					DEVNAME (curdev), strerror (errno));
-				return (-1);
-			}
-			if (write (fd, &obsolete_flag, sizeof (obsolete_flag)) !=
-				sizeof (obsolete_flag)) {
-				fprintf (stderr,
-					"Write error on %s: %s\n",
-					DEVNAME (curdev), strerror (errno));
-				return (-1);
-			}
-		}
-		printf ("Done\n");
-		printf ("Locking ...\n");
-		erase.length = DEVESIZE (otherdev);
-		erase.start = DEVOFFSET (otherdev);
-		ioctl (fdr, MEMLOCK, &erase);
-		if (HaveRedundEnv) {
-			erase.length = DEVESIZE (curdev);
-			erase.start = DEVOFFSET (curdev);
-			ioctl (fd, MEMLOCK, &erase);
-			if (close (fdr)) {
-				fprintf (stderr,
-					"I/O error on %s: %s\n",
-					DEVNAME (otherdev),
-					strerror (errno));
-				return (-1);
-			}
+		free (data);
+	}
+	if (HaveRedundEnv) {
+		/* change flag on current active env partition */
+		if (lseek (fd, DEVOFFSET (curdev) + sizeof (ulong), SEEK_SET)
+		    == -1) {
+			fprintf (stderr, "seek error on %s: %s\n",
+				 DEVNAME (curdev), strerror (errno));
+			return (-1);
 		}
-		printf ("Done\n");
-	} else {
-
-		if (lseek (fd, DEVOFFSET (curdev), SEEK_SET) == -1) {
+		if (write (fd, &obsolete_flag, sizeof (obsolete_flag)) !=
+		    sizeof (obsolete_flag)) {
 			fprintf (stderr,
-				"seek error on %s: %s\n",
-				DEVNAME (curdev), strerror (errno));
+				 "Write error on %s: %s\n",
+				 DEVNAME (curdev), strerror (errno));
 			return (-1);
 		}
-		if (read (fd, environment.image, CFG_ENV_SIZE) !=
-		    CFG_ENV_SIZE) {
+	}
+	printf ("Done\n");
+	printf ("Locking ...\n");
+	erase.length = DEVESIZE (otherdev);
+	erase.start = DEVOFFSET (otherdev);
+	ioctl (fdr, MEMLOCK, &erase);
+	if (HaveRedundEnv) {
+		erase.length = DEVESIZE (curdev);
+		erase.start = DEVOFFSET (curdev);
+		ioctl (fd, MEMLOCK, &erase);
+		if (close (fdr)) {
 			fprintf (stderr,
-				"Read error on %s: %s\n",
-				DEVNAME (curdev), strerror (errno));
+				 "I/O error on %s: %s\n",
+				 DEVNAME (otherdev),
+				 strerror (errno));
 			return (-1);
 		}
 	}
+	printf ("Done\n");
+
+	if (close (fd)) {
+		fprintf (stderr,
+			 "I/O error on %s: %s\n",
+			 DEVNAME (curdev), strerror (errno));
+		return (-1);
+	}
+
+	/* everything ok */
+	return (0);
+}
+
+static int flash_read (void)
+{
+	int fd;
+
+	if ((fd = open (DEVNAME (curdev), O_RDONLY)) < 0) {
+		fprintf (stderr,
+			 "Can't open %s: %s\n",
+			 DEVNAME (curdev), strerror (errno));
+		return (-1);
+	}
+
+	if (lseek (fd, DEVOFFSET (curdev), SEEK_SET) == -1) {
+		fprintf (stderr,
+			 "seek error on %s: %s\n",
+			 DEVNAME (curdev), strerror (errno));
+		return (-1);
+	}
+	if (read (fd, environment.image, CFG_ENV_SIZE) != CFG_ENV_SIZE) {
+		fprintf (stderr,
+			 "Read error on %s: %s\n",
+			 DEVNAME (curdev), strerror (errno));
+		return (-1);
+	}
 
 	if (close (fd)) {
 		fprintf (stderr,
-			"I/O error on %s: %s\n",
-			DEVNAME (curdev), strerror (errno));
+			 "I/O error on %s: %s\n",
+			 DEVNAME (curdev), strerror (errno));
 		return (-1);
 	}
 
@@ -621,7 +639,7 @@ static int env_init (void)
 	environment.data = HaveRedundEnv ? environment.image->redund.data :
 		environment.image->single.data;
 	curdev = 0;
-	if (flash_io (O_RDONLY)) {
+	if (flash_read ()) {
 		return (errno);
 	}
 
@@ -645,7 +663,7 @@ static int env_init (void)
 		}
 		environment.image = (union env_image *)addr2;
 
-		if (flash_io (O_RDONLY)) {
+		if (flash_read ()) {
 			return (errno);
 		}
 
-- 
1.5.4



More information about the U-Boot mailing list