All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems
@ 2018-03-09 12:12 Alex Kiernan
  2018-03-09 12:12 ` [U-Boot] [PATCH v2 1/4] tools: env: Pass through indent Alex Kiernan
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Alex Kiernan @ 2018-03-09 12:12 UTC (permalink / raw)
  To: u-boot


For environments stored on filesystems where you can't have a redundant
configuration, rather than just over-writing the existing environment
in fw_setenv, do the tradtional create temporary file, rename, sync,
sync directory dance to achieve ACID semantics when writing through
fw_setenv.

fw_env.c has had whitespace modified and HaveRedundEnv renamed in order
to avoid triggering large numbers of checkpatch warnings because of the
existing code style:

  warning: space prohibited between function name and open parenthesis '('
  check: Avoid CamelCase: <HaveRedundEnv>

Changes in v2:
- Clean style violations in existing code
- Acommodate white space changes from predecessor commit
- Use mkstemp() to create temporary filename, not a fixed one

Alex Kiernan (4):
  tools: env: Pass through indent
  tools: env: Fix CamelCasing style violation
  tools: env: Refactor write path of flash_io()
  tools: env: Implement atomic replace for filesystem

 tools/env/fw_env.c | 483 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 281 insertions(+), 202 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 1/4] tools: env: Pass through indent
  2018-03-09 12:12 [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Alex Kiernan
@ 2018-03-09 12:12 ` Alex Kiernan
  2018-03-19 22:35   ` [U-Boot] [U-Boot,v2,1/4] " Tom Rini
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 2/4] tools: env: Fix CamelCasing style violation Alex Kiernan
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Kiernan @ 2018-03-09 12:12 UTC (permalink / raw)
  To: u-boot

Pass tools/env/fw_env.c through indent to correct style violations. This
commit consists of only one non-whitespace change:

  tools/env/fw_env.c:549: error: do not use assignment in if condition

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

Changes in v2:
- Clean style violations in existing code

 tools/env/fw_env.c | 346 ++++++++++++++++++++++++++---------------------------
 1 file changed, 170 insertions(+), 176 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 0e3e343..9d5ccfc 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -64,14 +64,14 @@ struct envdev_s {
 	int is_ubi;			/* set if we use UBI volume */
 };
 
-static struct envdev_s envdevices[2] =
-{
+static struct envdev_s envdevices[2] = {
 	{
 		.mtd_type = MTD_ABSENT,
 	}, {
 		.mtd_type = MTD_ABSENT,
 	},
 };
+
 static int dev_current;
 
 #define DEVNAME(i)    envdevices[(i)].devname
@@ -88,14 +88,14 @@ static unsigned long usable_envsize;
 #define ENV_SIZE      usable_envsize
 
 struct env_image_single {
-	uint32_t	crc;	/* CRC32 over data bytes    */
-	char		data[];
+	uint32_t crc;		/* CRC32 over data bytes    */
+	char data[];
 };
 
 struct env_image_redundant {
-	uint32_t	crc;	/* CRC32 over data bytes    */
-	unsigned char	flags;	/* active or obsolete */
-	char		data[];
+	uint32_t crc;		/* CRC32 over data bytes    */
+	unsigned char flags;	/* active or obsolete */
+	char data[];
 };
 
 enum flag_scheme {
@@ -105,11 +105,11 @@ enum flag_scheme {
 };
 
 struct environment {
-	void			*image;
-	uint32_t		*crc;
-	unsigned char		*flags;
-	char			*data;
-	enum flag_scheme	flag_scheme;
+	void *image;
+	uint32_t *crc;
+	unsigned char *flags;
+	char *data;
+	enum flag_scheme flag_scheme;
 };
 
 static struct environment environment = {
@@ -347,11 +347,11 @@ static int ubi_write(int fd, const void *buf, size_t count)
 	return 0;
 }
 
-static int flash_io (int mode);
+static int flash_io(int mode);
 static int parse_config(struct env_opts *opts);
 
 #if defined(CONFIG_FILE)
-static int get_config (char *);
+static int get_config(char *);
 #endif
 
 static char *skip_chars(char *s)
@@ -394,7 +394,7 @@ static char *envmatch(char *s1, char *s2)
  * Search the environment for a variable.
  * Return the value, if found, or NULL, if not found.
  */
-char *fw_getenv (char *name)
+char *fw_getenv(char *name)
 {
 	char *env, *nxt;
 
@@ -403,12 +403,12 @@ char *fw_getenv (char *name)
 
 		for (nxt = env; *nxt; ++nxt) {
 			if (nxt >= &environment.data[ENV_SIZE]) {
-				fprintf (stderr, "## Error: "
+				fprintf(stderr, "## Error: "
 					"environment not terminated\n");
 				return NULL;
 			}
 		}
-		val = envmatch (name, env);
+		val = envmatch(name, env);
 		if (!val)
 			continue;
 		return val;
@@ -462,18 +462,18 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 	if (fw_env_open(opts))
 		return -1;
 
-	if (argc == 0) {		/* Print all env variables  */
+	if (argc == 0) {	/* Print all env variables  */
 		char *env, *nxt;
 		for (env = environment.data; *env; env = nxt + 1) {
 			for (nxt = env; *nxt; ++nxt) {
 				if (nxt >= &environment.data[ENV_SIZE]) {
-					fprintf (stderr, "## Error: "
+					fprintf(stderr, "## Error: "
 						"environment not terminated\n");
 					return -1;
 				}
 			}
 
-			printf ("%s\n", env);
+			printf("%s\n", env);
 		}
 		fw_env_close(opts);
 		return 0;
@@ -485,7 +485,7 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 
 		val = fw_getenv(name);
 		if (!val) {
-			fprintf (stderr, "## Error: \"%s\" not defined\n", name);
+			fprintf(stderr, "## Error: \"%s\" not defined\n", name);
 			rc = -1;
 			continue;
 		}
@@ -515,15 +515,13 @@ int fw_env_flush(struct env_opts *opts)
 
 	/* write environment back to flash */
 	if (flash_io(O_RDWR)) {
-		fprintf(stderr,
-			"Error: can't write fw_env to flash\n");
-			return -1;
+		fprintf(stderr, "Error: can't write fw_env to flash\n");
+		return -1;
 	}
 
 	return 0;
 }
 
-
 /*
  * Set/Clear a single variable in the environment.
  * This is called in sequence to update the environment
@@ -548,7 +546,8 @@ int fw_env_write(char *name, char *value)
 				return -1;
 			}
 		}
-		if ((oldval = envmatch (name, env)) != NULL)
+		oldval = envmatch(name, env);
+		if (oldval)
 			break;
 	}
 
@@ -571,7 +570,7 @@ int fw_env_write(char *name, char *value)
 			errno = EROFS;
 			return -1;
 		} else if (env_flags_validate_varaccess(name,
-		    ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR)) {
+			   ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR)) {
 			const char *defval = fw_getdefenv(name);
 
 			if (defval == NULL)
@@ -615,21 +614,21 @@ int fw_env_write(char *name, char *value)
 	/*
 	 * Append new definition at the end
 	 */
-	for (env = environment.data; *env || *(env + 1); ++env);
+	for (env = environment.data; *env || *(env + 1); ++env)
+		;
 	if (env > environment.data)
 		++env;
 	/*
 	 * Overflow when:
 	 * "name" + "=" + "val" +"\0\0"  > CUR_ENVSIZE - (env-environment)
 	 */
-	len = strlen (name) + 2;
+	len = strlen(name) + 2;
 	/* add '=' for first arg, ' ' for all others */
 	len += strlen(value) + 1;
 
 	if (len > (&environment.data[ENV_SIZE] - env)) {
-		fprintf (stderr,
-			"Error: environment overflow, \"%s\" deleted\n",
-			name);
+		fprintf(stderr,
+			"Error: environment overflow, \"%s\" deleted\n", name);
 		return -1;
 	}
 
@@ -759,7 +758,7 @@ int fw_parse_script(char *fname, struct env_opts *opts)
 		fp = fopen(fname, "r");
 		if (fp == NULL) {
 			fprintf(stderr, "I cannot open %s for reading\n",
-				 fname);
+				fname);
 			return -1;
 		}
 	}
@@ -774,7 +773,7 @@ int fw_parse_script(char *fname, struct env_opts *opts)
 		 */
 		if (dump[len - 1] != '\n') {
 			fprintf(stderr,
-			"Line %d not corrected terminated or too long\n",
+				"Line %d not corrected terminated or too long\n",
 				lineno);
 			ret = -1;
 			break;
@@ -807,7 +806,6 @@ int fw_parse_script(char *fname, struct env_opts *opts)
 			else
 				val = NULL;
 		}
-
 #ifdef DEBUG
 		fprintf(stderr, "Setting %s : %s\n",
 			name, val ? val : " removed");
@@ -824,7 +822,7 @@ int fw_parse_script(char *fname, struct env_opts *opts)
 		 */
 		if (fw_env_write(name, val)) {
 			fprintf(stderr,
-			"fw_env_write returns with error : %s\n",
+				"fw_env_write returns with error : %s\n",
 				strerror(errno));
 			ret = -1;
 			break;
@@ -867,13 +865,13 @@ static int flash_bad_block(int fd, uint8_t mtd_type, loff_t blockstart)
 		int badblock = ioctl(fd, MEMGETBADBLOCK, &blockstart);
 
 		if (badblock < 0) {
-			perror ("Cannot read bad block mark");
+			perror("Cannot read bad block mark");
 			return badblock;
 		}
 
 		if (badblock) {
 #ifdef DEBUG
-			fprintf (stderr, "Bad block at 0x%llx, skipping\n",
+			fprintf(stderr, "Bad block at 0x%llx, skipping\n",
 				(unsigned long long)blockstart);
 #endif
 			return badblock;
@@ -888,8 +886,8 @@ static int flash_bad_block(int fd, uint8_t mtd_type, loff_t blockstart)
  * bad blocks but makes sure it stays within ENVSECTORS (dev) starting from
  * the DEVOFFSET (dev) block. On NOR the loop is only run once.
  */
-static int flash_read_buf (int dev, int fd, void *buf, size_t count,
-			   off_t offset)
+static int flash_read_buf(int dev, int fd, void *buf, size_t count,
+			  off_t offset)
 {
 	size_t blocklen;	/* erase / write length - one block on NAND,
 				   0 on NOR */
@@ -901,7 +899,7 @@ static int flash_read_buf (int dev, int fd, void *buf, size_t count,
 				   MEMGETBADBLOCK needs 64 bits */
 	int rc;
 
-	blockstart = (offset / DEVESIZE (dev)) * DEVESIZE (dev);
+	blockstart = (offset / DEVESIZE(dev)) * DEVESIZE(dev);
 
 	/* Offset inside a block */
 	block_seek = offset - blockstart;
@@ -911,7 +909,7 @@ static int flash_read_buf (int dev, int fd, void *buf, size_t count,
 		 * NAND: calculate which blocks we are reading. We have
 		 * to read one block@a time to skip bad blocks.
 		 */
-		blocklen = DEVESIZE (dev);
+		blocklen = DEVESIZE(dev);
 
 		/* Limit to one block for the first read */
 		if (readlen > blocklen - block_seek)
@@ -923,17 +921,16 @@ static int flash_read_buf (int dev, int fd, void *buf, size_t count,
 	/* This only runs once on NOR flash */
 	while (processed < count) {
 		rc = flash_bad_block(fd, DEVTYPE(dev), blockstart);
-		if (rc < 0)		/* block test failed */
+		if (rc < 0)	/* block test failed */
 			return -1;
 
 		if (blockstart + block_seek + readlen > environment_end(dev)) {
 			/* End of range is reached */
-			fprintf (stderr,
-				 "Too few good blocks within range\n");
+			fprintf(stderr, "Too few good blocks within range\n");
 			return -1;
 		}
 
-		if (rc) {		/* block is bad */
+		if (rc) {	/* block is bad */
 			blockstart += blocklen;
 			continue;
 		}
@@ -942,21 +939,21 @@ static int flash_read_buf (int dev, int fd, void *buf, size_t count,
 		 * If a block is bad, we retry in the next block at the same
 		 * offset - see env/nand.c::writeenv()
 		 */
-		lseek (fd, blockstart + block_seek, SEEK_SET);
+		lseek(fd, blockstart + block_seek, SEEK_SET);
 
-		rc = read (fd, buf + processed, readlen);
+		rc = read(fd, buf + processed, readlen);
 		if (rc != readlen) {
-			fprintf (stderr, "Read error on %s: %s\n",
-				 DEVNAME (dev), strerror (errno));
+			fprintf(stderr, "Read error on %s: %s\n",
+				DEVNAME(dev), strerror(errno));
 			return -1;
 		}
 #ifdef DEBUG
 		fprintf(stderr, "Read 0x%x bytes at 0x%llx on %s\n",
-			rc, (unsigned long long) blockstart + block_seek,
+			rc, (unsigned long long)blockstart + block_seek,
 			DEVNAME(dev));
 #endif
 		processed += readlen;
-		readlen = min (blocklen, count - processed);
+		readlen = min(blocklen, count - processed);
 		block_seek = 0;
 		blockstart += blocklen;
 	}
@@ -1018,7 +1015,7 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 		 * to the end of the block
 		 */
 		write_total = ((block_seek + count + blocklen - 1) /
-							blocklen) * blocklen;
+			       blocklen) * blocklen;
 	}
 
 	/*
@@ -1027,11 +1024,11 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 	 * block back again.
 	 */
 	if (write_total > count) {
-		data = malloc (erase_len);
+		data = malloc(erase_len);
 		if (!data) {
-			fprintf (stderr,
-				 "Cannot malloc %zu bytes: %s\n",
-				 erase_len, strerror (errno));
+			fprintf(stderr,
+				"Cannot malloc %zu bytes: %s\n",
+				erase_len, strerror(errno));
 			return -1;
 		}
 
@@ -1047,13 +1044,13 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 			if (block_seek != 0)
 				fprintf(stderr, " and ");
 			fprintf(stderr, "0x%lx - 0x%lx",
-				(unsigned long) block_seek + count,
-				(unsigned long) write_total - 1);
+				(unsigned long)block_seek + count,
+				(unsigned long)write_total - 1);
 		}
 		fprintf(stderr, "\n");
 #endif
 		/* Overwrite the old environment */
-		memcpy (data + block_seek, buf, count);
+		memcpy(data + block_seek, buf, count);
 	} else {
 		/*
 		 * We get here, iff offset is block-aligned and count is a
@@ -1077,15 +1074,15 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 	/* This only runs once on NOR flash and SPI-dataflash */
 	while (processed < write_total) {
 		rc = flash_bad_block(fd, DEVTYPE(dev), blockstart);
-		if (rc < 0)		/* block test failed */
+		if (rc < 0)	/* block test failed */
 			return rc;
 
 		if (blockstart + erasesize > environment_end(dev)) {
-			fprintf (stderr, "End of range reached, aborting\n");
+			fprintf(stderr, "End of range reached, aborting\n");
 			return -1;
 		}
 
-		if (rc) {		/* block is bad */
+		if (rc) {	/* block is bad */
 			blockstart += blocklen;
 			continue;
 		}
@@ -1103,34 +1100,33 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 				}
 		}
 
-		if (lseek (fd, blockstart, SEEK_SET) == -1) {
-			fprintf (stderr,
-				 "Seek error on %s: %s\n",
-				 DEVNAME (dev), strerror (errno));
+		if (lseek(fd, blockstart, SEEK_SET) == -1) {
+			fprintf(stderr,
+				"Seek error on %s: %s\n",
+				DEVNAME(dev), strerror(errno));
 			return -1;
 		}
-
 #ifdef DEBUG
 		fprintf(stderr, "Write 0x%llx bytes at 0x%llx\n",
-			(unsigned long long) erasesize,
-			(unsigned long long) blockstart);
+			(unsigned long long)erasesize,
+			(unsigned long long)blockstart);
 #endif
-		if (write (fd, data + processed, erasesize) != erasesize) {
-			fprintf (stderr, "Write error on %s: %s\n",
-				 DEVNAME (dev), strerror (errno));
+		if (write(fd, data + processed, erasesize) != erasesize) {
+			fprintf(stderr, "Write error on %s: %s\n",
+				DEVNAME(dev), strerror(errno));
 			return -1;
 		}
 
 		if (DEVTYPE(dev) != MTD_ABSENT)
 			ioctl(fd, MEMLOCK, &erase);
 
-		processed  += erasesize;
+		processed += erasesize;
 		block_seek = 0;
 		blockstart += erasesize;
 	}
 
 	if (write_total > count)
-		free (data);
+		free(data);
 
 	return processed;
 }
@@ -1138,30 +1134,30 @@ static int flash_write_buf(int dev, int fd, void *buf, size_t count)
 /*
  * Set obsolete flag at offset - NOR flash only
  */
-static int flash_flag_obsolete (int dev, int fd, off_t offset)
+static int flash_flag_obsolete(int dev, int fd, off_t offset)
 {
 	int rc;
 	struct erase_info_user erase;
 
-	erase.start  = DEVOFFSET (dev);
-	erase.length = DEVESIZE (dev);
+	erase.start = DEVOFFSET(dev);
+	erase.length = DEVESIZE(dev);
 	/* This relies on the fact, that obsolete_flag == 0 */
-	rc = lseek (fd, offset, SEEK_SET);
+	rc = lseek(fd, offset, SEEK_SET);
 	if (rc < 0) {
-		fprintf (stderr, "Cannot seek to set the flag on %s \n",
-			 DEVNAME (dev));
+		fprintf(stderr, "Cannot seek to set the flag on %s\n",
+			DEVNAME(dev));
 		return rc;
 	}
-	ioctl (fd, MEMUNLOCK, &erase);
-	rc = write (fd, &obsolete_flag, sizeof (obsolete_flag));
-	ioctl (fd, MEMLOCK, &erase);
+	ioctl(fd, MEMUNLOCK, &erase);
+	rc = write(fd, &obsolete_flag, sizeof(obsolete_flag));
+	ioctl(fd, MEMLOCK, &erase);
 	if (rc < 0)
-		perror ("Could not set obsolete flag");
+		perror("Could not set obsolete flag");
 
 	return rc;
 }
 
-static int flash_write (int fd_current, int fd_target, int dev_target)
+static int flash_write(int fd_current, int fd_target, int dev_target)
 {
 	int rc;
 
@@ -1175,14 +1171,14 @@ static int flash_write (int fd_current, int fd_target, int dev_target)
 		*environment.flags = active_flag;
 		break;
 	default:
-		fprintf (stderr, "Unimplemented flash scheme %u \n",
-			 environment.flag_scheme);
+		fprintf(stderr, "Unimplemented flash scheme %u\n",
+			environment.flag_scheme);
 		return -1;
 	}
 
 #ifdef DEBUG
 	fprintf(stderr, "Writing new environment at 0x%llx on %s\n",
-		DEVOFFSET (dev_target), DEVNAME (dev_target));
+		DEVOFFSET(dev_target), DEVNAME(dev_target));
 #endif
 
 	if (IS_UBI(dev_target)) {
@@ -1198,20 +1194,20 @@ static int flash_write (int fd_current, int fd_target, int dev_target)
 
 	if (environment.flag_scheme == FLAG_BOOLEAN) {
 		/* Have to set obsolete flag */
-		off_t offset = DEVOFFSET (dev_current) +
-			offsetof (struct env_image_redundant, flags);
+		off_t offset = DEVOFFSET(dev_current) +
+		    offsetof(struct env_image_redundant, flags);
 #ifdef DEBUG
 		fprintf(stderr,
 			"Setting obsolete flag in environment@0x%llx on %s\n",
-			DEVOFFSET (dev_current), DEVNAME (dev_current));
+			DEVOFFSET(dev_current), DEVNAME(dev_current));
 #endif
-		flash_flag_obsolete (dev_current, fd_current, offset);
+		flash_flag_obsolete(dev_current, fd_current, offset);
 	}
 
 	return 0;
 }
 
-static int flash_read (int fd)
+static int flash_read(int fd)
 {
 	int rc;
 
@@ -1229,16 +1225,16 @@ static int flash_read (int fd)
 	return 0;
 }
 
-static int flash_io (int mode)
+static int flash_io(int mode)
 {
 	int fd_current, fd_target, rc, dev_target;
 
 	/* dev_current: fd_current, erase_current */
-	fd_current = open (DEVNAME (dev_current), mode);
+	fd_current = open(DEVNAME(dev_current), mode);
 	if (fd_current < 0) {
-		fprintf (stderr,
-			 "Can't open %s: %s\n",
-			 DEVNAME (dev_current), strerror (errno));
+		fprintf(stderr,
+			"Can't open %s: %s\n",
+			DEVNAME(dev_current), strerror(errno));
 		return -1;
 	}
 
@@ -1247,12 +1243,11 @@ static int flash_io (int mode)
 			/* switch to next partition for writing */
 			dev_target = !dev_current;
 			/* dev_target: fd_target, erase_target */
-			fd_target = open (DEVNAME (dev_target), mode);
+			fd_target = open(DEVNAME(dev_target), mode);
 			if (fd_target < 0) {
-				fprintf (stderr,
-					 "Can't open %s: %s\n",
-					 DEVNAME (dev_target),
-					 strerror (errno));
+				fprintf(stderr,
+					"Can't open %s: %s\n",
+					DEVNAME(dev_target), strerror(errno));
 				rc = -1;
 				goto exit;
 			}
@@ -1261,40 +1256,38 @@ static int flash_io (int mode)
 			fd_target = fd_current;
 		}
 
-		rc = flash_write (fd_current, fd_target, dev_target);
+		rc = flash_write(fd_current, fd_target, dev_target);
 
-		if (fsync(fd_current) &&
-		    !(errno == EINVAL || errno == EROFS)) {
-			fprintf (stderr,
-				 "fsync failed on %s: %s\n",
-				 DEVNAME (dev_current), strerror (errno));
+		if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
+			fprintf(stderr,
+				"fsync failed on %s: %s\n",
+				DEVNAME(dev_current), strerror(errno));
 		}
 
 		if (HaveRedundEnv) {
 			if (fsync(fd_target) &&
 			    !(errno == EINVAL || errno == EROFS)) {
-				fprintf (stderr,
-					 "fsync failed on %s: %s\n",
-					 DEVNAME (dev_current), strerror (errno));
+				fprintf(stderr,
+					"fsync failed on %s: %s\n",
+					DEVNAME(dev_current), strerror(errno));
 			}
 
-			if (close (fd_target)) {
-				fprintf (stderr,
+			if (close(fd_target)) {
+				fprintf(stderr,
 					"I/O error on %s: %s\n",
-					DEVNAME (dev_target),
-					strerror (errno));
+					DEVNAME(dev_target), strerror(errno));
 				rc = -1;
 			}
 		}
 	} else {
-		rc = flash_read (fd_current);
+		rc = flash_read(fd_current);
 	}
 
-exit:
-	if (close (fd_current)) {
-		fprintf (stderr,
-			 "I/O error on %s: %s\n",
-			 DEVNAME (dev_current), strerror (errno));
+ exit:
+	if (close(fd_current)) {
+		fprintf(stderr,
+			"I/O error on %s: %s\n",
+			DEVNAME(dev_current), strerror(errno));
 		return -1;
 	}
 
@@ -1322,7 +1315,7 @@ int fw_env_open(struct env_opts *opts)
 	if (!opts)
 		opts = &default_opts;
 
-	if (parse_config(opts))		/* should fill envdevices */
+	if (parse_config(opts))	/* should fill envdevices */
 		return -EINVAL;
 
 	addr0 = calloc(1, CUR_ENVSIZE);
@@ -1339,14 +1332,14 @@ int fw_env_open(struct env_opts *opts)
 
 	if (HaveRedundEnv) {
 		redundant = addr0;
-		environment.crc		= &redundant->crc;
-		environment.flags	= &redundant->flags;
-		environment.data	= redundant->data;
+		environment.crc = &redundant->crc;
+		environment.flags = &redundant->flags;
+		environment.data = redundant->data;
 	} else {
 		single = addr0;
-		environment.crc		= &single->crc;
-		environment.flags	= NULL;
-		environment.data	= single->data;
+		environment.crc = &single->crc;
+		environment.flags = NULL;
+		environment.data = single->data;
 	}
 
 	dev_current = 0;
@@ -1355,14 +1348,15 @@ int fw_env_open(struct env_opts *opts)
 		goto open_cleanup;
 	}
 
-	crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
+	crc0 = crc32(0, (uint8_t *)environment.data, ENV_SIZE);
 
 	crc0_ok = (crc0 == *environment.crc);
 	if (!HaveRedundEnv) {
 		if (!crc0_ok) {
-			fprintf (stderr,
+			fprintf(stderr,
 				"Warning: Bad CRC, using default environment\n");
-			memcpy(environment.data, default_environment, sizeof default_environment);
+			memcpy(environment.data, default_environment,
+			       sizeof(default_environment));
 		}
 	} else {
 		flag0 = *environment.flags;
@@ -1406,12 +1400,12 @@ int fw_env_open(struct env_opts *opts)
 			   IS_UBI(dev_current) == IS_UBI(!dev_current)) {
 			environment.flag_scheme = FLAG_INCREMENTAL;
 		} else {
-			fprintf (stderr, "Incompatible flash types!\n");
+			fprintf(stderr, "Incompatible flash types!\n");
 			ret = -EINVAL;
 			goto open_cleanup;
 		}
 
-		crc1 = crc32 (0, (uint8_t *) redundant->data, ENV_SIZE);
+		crc1 = crc32(0, (uint8_t *)redundant->data, ENV_SIZE);
 
 		crc1_ok = (crc1 == redundant->crc);
 		flag1 = redundant->flags;
@@ -1421,10 +1415,10 @@ int fw_env_open(struct env_opts *opts)
 		} else if (!crc0_ok && crc1_ok) {
 			dev_current = 1;
 		} else if (!crc0_ok && !crc1_ok) {
-			fprintf (stderr,
+			fprintf(stderr,
 				"Warning: Bad CRC, using default environment\n");
-			memcpy (environment.data, default_environment,
-				sizeof default_environment);
+			memcpy(environment.data, default_environment,
+			       sizeof(default_environment));
 			dev_current = 0;
 		} else {
 			switch (environment.flag_scheme) {
@@ -1451,12 +1445,12 @@ int fw_env_open(struct env_opts *opts)
 				else if ((flag1 == 255 && flag0 == 0) ||
 					 flag0 >= flag1)
 					dev_current = 0;
-				else /* flag1 > flag0 */
+				else	/* flag1 > flag0 */
 					dev_current = 1;
 				break;
 			default:
-				fprintf (stderr, "Unknown flag scheme %u \n",
-					 environment.flag_scheme);
+				fprintf(stderr, "Unknown flag scheme %u\n",
+					environment.flag_scheme);
 				return -1;
 			}
 		}
@@ -1467,15 +1461,15 @@ int fw_env_open(struct env_opts *opts)
 		 * flags before writing out
 		 */
 		if (dev_current) {
-			environment.image	= addr1;
-			environment.crc		= &redundant->crc;
-			environment.flags	= &redundant->flags;
-			environment.data	= redundant->data;
-			free (addr0);
+			environment.image = addr1;
+			environment.crc = &redundant->crc;
+			environment.flags = &redundant->flags;
+			environment.data = redundant->data;
+			free(addr0);
 		} else {
-			environment.image	= addr0;
+			environment.image = addr0;
 			/* Other pointers are already set */
-			free (addr1);
+			free(addr1);
 		}
 #ifdef DEBUG
 		fprintf(stderr, "Selected env in %s\n", DEVNAME(dev_current));
@@ -1483,7 +1477,7 @@ int fw_env_open(struct env_opts *opts)
 	}
 	return 0;
 
-open_cleanup:
+ open_cleanup:
 	if (addr0)
 		free(addr0);
 
@@ -1518,15 +1512,13 @@ static int check_device_config(int dev)
 	fd = open(DEVNAME(dev), O_RDONLY);
 	if (fd < 0) {
 		fprintf(stderr,
-			"Cannot open %s: %s\n",
-			DEVNAME(dev), strerror(errno));
+			"Cannot open %s: %s\n", DEVNAME(dev), strerror(errno));
 		return -1;
 	}
 
 	rc = fstat(fd, &st);
 	if (rc < 0) {
-		fprintf(stderr, "Cannot stat the file %s\n",
-			DEVNAME(dev));
+		fprintf(stderr, "Cannot stat the file %s\n", DEVNAME(dev));
 		goto err;
 	}
 
@@ -1571,14 +1563,16 @@ static int check_device_config(int dev)
 		if (DEVOFFSET(dev) < 0) {
 			rc = ioctl(fd, BLKGETSIZE64, &size);
 			if (rc < 0) {
-				fprintf(stderr, "Could not get block device size on %s\n",
+				fprintf(stderr,
+					"Could not get block device size on %s\n",
 					DEVNAME(dev));
 				goto err;
 			}
 
 			DEVOFFSET(dev) = DEVOFFSET(dev) + size;
 #ifdef DEBUG
-			fprintf(stderr, "Calculated device offset 0x%llx on %s\n",
+			fprintf(stderr,
+				"Calculated device offset 0x%llx on %s\n",
 				DEVOFFSET(dev), DEVNAME(dev));
 #endif
 		}
@@ -1589,18 +1583,20 @@ static int check_device_config(int dev)
 		ENVSECTORS(dev) = DIV_ROUND_UP(ENVSIZE(dev), DEVESIZE(dev));
 
 	if (DEVOFFSET(dev) % DEVESIZE(dev) != 0) {
-		fprintf(stderr, "Environment does not start on (erase) block boundary\n");
+		fprintf(stderr,
+			"Environment does not start on (erase) block boundary\n");
 		errno = EINVAL;
 		return -1;
 	}
 
 	if (ENVSIZE(dev) > ENVSECTORS(dev) * DEVESIZE(dev)) {
-		fprintf(stderr, "Environment does not fit into available sectors\n");
+		fprintf(stderr,
+			"Environment does not fit into available sectors\n");
 		errno = EINVAL;
 		return -1;
 	}
 
-err:
+ err:
 	close(fd);
 	return rc;
 }
@@ -1620,33 +1616,33 @@ static int parse_config(struct env_opts *opts)
 		return -1;
 	}
 #else
-	DEVNAME (0) = DEVICE1_NAME;
-	DEVOFFSET (0) = DEVICE1_OFFSET;
-	ENVSIZE (0) = ENV1_SIZE;
+	DEVNAME(0) = DEVICE1_NAME;
+	DEVOFFSET(0) = DEVICE1_OFFSET;
+	ENVSIZE(0) = ENV1_SIZE;
 
 	/* Set defaults for DEVESIZE, ENVSECTORS later once we
 	 * know DEVTYPE
 	 */
 #ifdef DEVICE1_ESIZE
-	DEVESIZE (0) = DEVICE1_ESIZE;
+	DEVESIZE(0) = DEVICE1_ESIZE;
 #endif
 #ifdef DEVICE1_ENVSECTORS
-	ENVSECTORS (0) = DEVICE1_ENVSECTORS;
+	ENVSECTORS(0) = DEVICE1_ENVSECTORS;
 #endif
 
 #ifdef HAVE_REDUND
-	DEVNAME (1) = DEVICE2_NAME;
-	DEVOFFSET (1) = DEVICE2_OFFSET;
-	ENVSIZE (1) = ENV2_SIZE;
+	DEVNAME(1) = DEVICE2_NAME;
+	DEVOFFSET(1) = DEVICE2_OFFSET;
+	ENVSIZE(1) = ENV2_SIZE;
 
 	/* Set defaults for DEVESIZE, ENVSECTORS later once we
 	 * know DEVTYPE
 	 */
 #ifdef DEVICE2_ESIZE
-	DEVESIZE (1) = DEVICE2_ESIZE;
+	DEVESIZE(1) = DEVICE2_ESIZE;
 #endif
 #ifdef DEVICE2_ENVSECTORS
-	ENVSECTORS (1) = DEVICE2_ENVSECTORS;
+	ENVSECTORS(1) = DEVICE2_ENVSECTORS;
 #endif
 	HaveRedundEnv = 1;
 #endif
@@ -1675,7 +1671,7 @@ static int parse_config(struct env_opts *opts)
 }
 
 #if defined(CONFIG_FILE)
-static int get_config (char *fname)
+static int get_config(char *fname)
 {
 	FILE *fp;
 	int i = 0;
@@ -1683,11 +1679,11 @@ static int get_config (char *fname)
 	char dump[128];
 	char *devname;
 
-	fp = fopen (fname, "r");
+	fp = fopen(fname, "r");
 	if (fp == NULL)
 		return -1;
 
-	while (i < 2 && fgets (dump, sizeof (dump), fp)) {
+	while (i < 2 && fgets(dump, sizeof(dump), fp)) {
 		/* Skip incomplete conversions and comment strings */
 		if (dump[0] == '#')
 			continue;
@@ -1695,9 +1691,7 @@ static int get_config (char *fname)
 		rc = sscanf(dump, "%ms %lli %lx %lx %lx",
 			    &devname,
 			    &DEVOFFSET(i),
-			    &ENVSIZE(i),
-			    &DEVESIZE(i),
-			    &ENVSECTORS(i));
+			    &ENVSIZE(i), &DEVESIZE(i), &ENVSECTORS(i));
 
 		if (rc < 3)
 			continue;
@@ -1710,10 +1704,10 @@ static int get_config (char *fname)
 
 		i++;
 	}
-	fclose (fp);
+	fclose(fp);
 
 	HaveRedundEnv = i - 1;
-	if (!i) {			/* No valid entries found */
+	if (!i) {		/* No valid entries found */
 		errno = EINVAL;
 		return -1;
 	} else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 2/4] tools: env: Fix CamelCasing style violation
  2018-03-09 12:12 [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Alex Kiernan
  2018-03-09 12:12 ` [U-Boot] [PATCH v2 1/4] tools: env: Pass through indent Alex Kiernan
@ 2018-03-09 12:13 ` Alex Kiernan
  2018-03-19 22:35   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 3/4] tools: env: Refactor write path of flash_io() Alex Kiernan
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Kiernan @ 2018-03-09 12:13 UTC (permalink / raw)
  To: u-boot

Replace HaveRedundEnv with have_redund_env to fix style violation.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

Changes in v2:
- new

 tools/env/fw_env.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 9d5ccfc..f3bfee4 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -116,7 +116,7 @@ static struct environment environment = {
 	.flag_scheme = FLAG_NONE,
 };
 
-static int HaveRedundEnv = 0;
+static int have_redund_env;
 
 static unsigned char active_flag = 1;
 /* obsolete_flag must be 0 to efficiently set it on NOR flash without erasing */
@@ -1239,7 +1239,7 @@ static int flash_io(int mode)
 	}
 
 	if (mode == O_RDWR) {
-		if (HaveRedundEnv) {
+		if (have_redund_env) {
 			/* switch to next partition for writing */
 			dev_target = !dev_current;
 			/* dev_target: fd_target, erase_target */
@@ -1264,7 +1264,7 @@ static int flash_io(int mode)
 				DEVNAME(dev_current), strerror(errno));
 		}
 
-		if (HaveRedundEnv) {
+		if (have_redund_env) {
 			if (fsync(fd_target) &&
 			    !(errno == EINVAL || errno == EROFS)) {
 				fprintf(stderr,
@@ -1330,7 +1330,7 @@ int fw_env_open(struct env_opts *opts)
 	/* read environment from FLASH to local buffer */
 	environment.image = addr0;
 
-	if (HaveRedundEnv) {
+	if (have_redund_env) {
 		redundant = addr0;
 		environment.crc = &redundant->crc;
 		environment.flags = &redundant->flags;
@@ -1351,7 +1351,7 @@ int fw_env_open(struct env_opts *opts)
 	crc0 = crc32(0, (uint8_t *)environment.data, ENV_SIZE);
 
 	crc0_ok = (crc0 == *environment.crc);
-	if (!HaveRedundEnv) {
+	if (!have_redund_env) {
 		if (!crc0_ok) {
 			fprintf(stderr,
 				"Warning: Bad CRC, using default environment\n");
@@ -1644,14 +1644,14 @@ static int parse_config(struct env_opts *opts)
 #ifdef DEVICE2_ENVSECTORS
 	ENVSECTORS(1) = DEVICE2_ENVSECTORS;
 #endif
-	HaveRedundEnv = 1;
+	have_redund_env = 1;
 #endif
 #endif
 	rc = check_device_config(0);
 	if (rc < 0)
 		return rc;
 
-	if (HaveRedundEnv) {
+	if (have_redund_env) {
 		rc = check_device_config(1);
 		if (rc < 0)
 			return rc;
@@ -1664,7 +1664,7 @@ static int parse_config(struct env_opts *opts)
 	}
 
 	usable_envsize = CUR_ENVSIZE - sizeof(uint32_t);
-	if (HaveRedundEnv)
+	if (have_redund_env)
 		usable_envsize -= sizeof(char);
 
 	return 0;
@@ -1706,7 +1706,7 @@ static int get_config(char *fname)
 	}
 	fclose(fp);
 
-	HaveRedundEnv = i - 1;
+	have_redund_env = i - 1;
 	if (!i) {		/* No valid entries found */
 		errno = EINVAL;
 		return -1;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 3/4] tools: env: Refactor write path of flash_io()
  2018-03-09 12:12 [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Alex Kiernan
  2018-03-09 12:12 ` [U-Boot] [PATCH v2 1/4] tools: env: Pass through indent Alex Kiernan
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 2/4] tools: env: Fix CamelCasing style violation Alex Kiernan
@ 2018-03-09 12:13 ` Alex Kiernan
  2018-03-19 22:35   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 4/4] tools: env: Implement atomic replace for filesystem Alex Kiernan
  2018-03-09 12:30 ` [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Wolfgang Denk
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Kiernan @ 2018-03-09 12:13 UTC (permalink / raw)
  To: u-boot

Extract write path of flash_io() into a separate function. This patch
should be a functional no-op.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
Reviewed-by: Stefano Babic <sbabic@denx.de>
---

Changes in v2:
- Acommodate white space changes from predecessor commit

 tools/env/fw_env.c | 92 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index f3bfee4..600fe5d 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1225,9 +1225,57 @@ static int flash_read(int fd)
 	return 0;
 }
 
+static int flash_io_write(int fd_current)
+{
+	int fd_target, rc, dev_target;
+
+	if (have_redund_env) {
+		/* switch to next partition for writing */
+		dev_target = !dev_current;
+		/* dev_target: fd_target, erase_target */
+		fd_target = open(DEVNAME(dev_target), O_RDWR);
+		if (fd_target < 0) {
+			fprintf(stderr,
+				"Can't open %s: %s\n",
+				DEVNAME(dev_target), strerror(errno));
+			rc = -1;
+			goto exit;
+		}
+	} else {
+		dev_target = dev_current;
+		fd_target = fd_current;
+	}
+
+	rc = flash_write(fd_current, fd_target, dev_target);
+
+	if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
+		fprintf(stderr,
+			"fsync failed on %s: %s\n",
+			DEVNAME(dev_current), strerror(errno));
+	}
+
+	if (have_redund_env) {
+		if (fsync(fd_target) &&
+		    !(errno == EINVAL || errno == EROFS)) {
+			fprintf(stderr,
+				"fsync failed on %s: %s\n",
+				DEVNAME(dev_current), strerror(errno));
+		}
+
+		if (close(fd_target)) {
+			fprintf(stderr,
+				"I/O error on %s: %s\n",
+				DEVNAME(dev_target), strerror(errno));
+			rc = -1;
+		}
+	}
+ exit:
+	return rc;
+}
+
 static int flash_io(int mode)
 {
-	int fd_current, fd_target, rc, dev_target;
+	int fd_current, rc;
 
 	/* dev_current: fd_current, erase_current */
 	fd_current = open(DEVNAME(dev_current), mode);
@@ -1239,51 +1287,11 @@ static int flash_io(int mode)
 	}
 
 	if (mode == O_RDWR) {
-		if (have_redund_env) {
-			/* switch to next partition for writing */
-			dev_target = !dev_current;
-			/* dev_target: fd_target, erase_target */
-			fd_target = open(DEVNAME(dev_target), mode);
-			if (fd_target < 0) {
-				fprintf(stderr,
-					"Can't open %s: %s\n",
-					DEVNAME(dev_target), strerror(errno));
-				rc = -1;
-				goto exit;
-			}
-		} else {
-			dev_target = dev_current;
-			fd_target = fd_current;
-		}
-
-		rc = flash_write(fd_current, fd_target, dev_target);
-
-		if (fsync(fd_current) && !(errno == EINVAL || errno == EROFS)) {
-			fprintf(stderr,
-				"fsync failed on %s: %s\n",
-				DEVNAME(dev_current), strerror(errno));
-		}
-
-		if (have_redund_env) {
-			if (fsync(fd_target) &&
-			    !(errno == EINVAL || errno == EROFS)) {
-				fprintf(stderr,
-					"fsync failed on %s: %s\n",
-					DEVNAME(dev_current), strerror(errno));
-			}
-
-			if (close(fd_target)) {
-				fprintf(stderr,
-					"I/O error on %s: %s\n",
-					DEVNAME(dev_target), strerror(errno));
-				rc = -1;
-			}
-		}
+		rc = flash_io_write(fd_current);
 	} else {
 		rc = flash_read(fd_current);
 	}
 
- exit:
 	if (close(fd_current)) {
 		fprintf(stderr,
 			"I/O error on %s: %s\n",
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 4/4] tools: env: Implement atomic replace for filesystem
  2018-03-09 12:12 [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Alex Kiernan
                   ` (2 preceding siblings ...)
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 3/4] tools: env: Refactor write path of flash_io() Alex Kiernan
@ 2018-03-09 12:13 ` Alex Kiernan
  2018-03-19 22:35   ` [U-Boot] [U-Boot, v2, " Tom Rini
  2018-03-09 12:30 ` [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Wolfgang Denk
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Kiernan @ 2018-03-09 12:13 UTC (permalink / raw)
  To: u-boot

If the U-Boot environment is stored in a regular file and redundant
operation isn't set, then write to a temporary file and perform an
atomic rename.

Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---

Changes in v2:
- Use mkstemp() to create temporary filename, not a fixed one

 tools/env/fw_env.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 600fe5d..77eac3d 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -14,6 +14,7 @@
 #include <errno.h>
 #include <env_flags.h>
 #include <fcntl.h>
+#include <libgen.h>
 #include <linux/fs.h>
 #include <linux/stringify.h>
 #include <ctype.h>
@@ -1225,9 +1226,48 @@ static int flash_read(int fd)
 	return 0;
 }
 
+static int flash_open_tempfile(const char **dname, const char **target_temp)
+{
+	char *dup_name = strdup(DEVNAME(dev_current));
+	char *temp_name = NULL;
+	int rc = -1;
+
+	if (!dup_name)
+		return -1;
+
+	*dname = dirname(dup_name);
+	if (!*dname)
+		goto err;
+
+	rc = asprintf(&temp_name, "%s/XXXXXX", *dname);
+	if (rc == -1)
+		goto err;
+
+	rc = mkstemp(temp_name);
+	if (rc == -1) {
+		/* fall back to in place write */
+		fprintf(stderr,
+			"Can't create %s: %s\n", temp_name, strerror(errno));
+		free(temp_name);
+	} else {
+		*target_temp = temp_name;
+		/* deliberately leak dup_name as dname /might/ point into
+		 * it and we need it for our caller
+		 */
+		dup_name = NULL;
+	}
+
+err:
+	if (dup_name)
+		free(dup_name);
+
+	return rc;
+}
+
 static int flash_io_write(int fd_current)
 {
-	int fd_target, rc, dev_target;
+	int fd_target = -1, rc, dev_target;
+	const char *dname, *target_temp = NULL;
 
 	if (have_redund_env) {
 		/* switch to next partition for writing */
@@ -1242,8 +1282,17 @@ static int flash_io_write(int fd_current)
 			goto exit;
 		}
 	} else {
+		struct stat sb;
+
+		if (fstat(fd_current, &sb) == 0 && S_ISREG(sb.st_mode)) {
+			/* if any part of flash_open_tempfile() fails we fall
+			 * back to in-place writes
+			 */
+			fd_target = flash_open_tempfile(&dname, &target_temp);
+		}
 		dev_target = dev_current;
-		fd_target = fd_current;
+		if (fd_target == -1)
+			fd_target = fd_current;
 	}
 
 	rc = flash_write(fd_current, fd_target, dev_target);
@@ -1254,7 +1303,7 @@ static int flash_io_write(int fd_current)
 			DEVNAME(dev_current), strerror(errno));
 	}
 
-	if (have_redund_env) {
+	if (fd_current != fd_target) {
 		if (fsync(fd_target) &&
 		    !(errno == EINVAL || errno == EROFS)) {
 			fprintf(stderr,
@@ -1268,6 +1317,34 @@ static int flash_io_write(int fd_current)
 				DEVNAME(dev_target), strerror(errno));
 			rc = -1;
 		}
+
+		if (target_temp) {
+			int dir_fd;
+
+			dir_fd = open(dname, O_DIRECTORY | O_RDONLY);
+			if (dir_fd == -1)
+				fprintf(stderr,
+					"Can't open %s: %s\n",
+					dname, strerror(errno));
+
+			if (rename(target_temp, DEVNAME(dev_target))) {
+				fprintf(stderr,
+					"rename failed %s => %s: %s\n",
+					target_temp, DEVNAME(dev_target),
+					strerror(errno));
+				rc = -1;
+			}
+
+			if (dir_fd != -1 && fsync(dir_fd))
+				fprintf(stderr,
+					"fsync failed on %s: %s\n",
+					dname, strerror(errno));
+
+			if (dir_fd != -1 && close(dir_fd))
+				fprintf(stderr,
+					"I/O error on %s: %s\n",
+					dname, strerror(errno));
+		}
 	}
  exit:
 	return rc;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems
  2018-03-09 12:12 [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Alex Kiernan
                   ` (3 preceding siblings ...)
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 4/4] tools: env: Implement atomic replace for filesystem Alex Kiernan
@ 2018-03-09 12:30 ` Wolfgang Denk
  2018-03-09 12:51   ` Alex Kiernan
  4 siblings, 1 reply; 11+ messages in thread
From: Wolfgang Denk @ 2018-03-09 12:30 UTC (permalink / raw)
  To: u-boot

Dear Alex,

In message <1520597582-12979-1-git-send-email-alex.kiernan@gmail.com> you wrote:
> 
> For environments stored on filesystems where you can't have a redundant
> configuration, rather than just over-writing the existing environment
> in fw_setenv, do the tradtional create temporary file, rename, sync,
> sync directory dance to achieve ACID semantics when writing through
> fw_setenv.

I isagree with the statement that we _can't have_ redundant
environments in a file system.  It may not be supported by the
current implementation, but there is actually nothing that would
prevent us to come up with more powerful code that supports exactly
such a feature.

[This does not mean that I disagree with your patch - on contary, I
think it's good.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A person with one watch knows what time it  is;  a  person  with  two
watches is never sure.                                       Proverb

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems
  2018-03-09 12:30 ` [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Wolfgang Denk
@ 2018-03-09 12:51   ` Alex Kiernan
  0 siblings, 0 replies; 11+ messages in thread
From: Alex Kiernan @ 2018-03-09 12:51 UTC (permalink / raw)
  To: u-boot

On Fri, Mar 9, 2018 at 12:30 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Alex,
>
> In message <1520597582-12979-1-git-send-email-alex.kiernan@gmail.com> you wrote:
>>
>> For environments stored on filesystems where you can't have a redundant
>> configuration, rather than just over-writing the existing environment
>> in fw_setenv, do the tradtional create temporary file, rename, sync,
>> sync directory dance to achieve ACID semantics when writing through
>> fw_setenv.
>
> I isagree with the statement that we _can't have_ redundant

Yeah... sorry, sloppy wording on my part.

> environments in a file system.  It may not be supported by the
> current implementation, but there is actually nothing that would
> prevent us to come up with more powerful code that supports exactly
> such a feature.
>
> [This does not mean that I disagree with your patch - on contary, I
> think it's good.]
>

Thanks!

-- 
Alex Kiernan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot,v2,1/4] tools: env: Pass through indent
  2018-03-09 12:12 ` [U-Boot] [PATCH v2 1/4] tools: env: Pass through indent Alex Kiernan
@ 2018-03-19 22:35   ` Tom Rini
  0 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2018-03-19 22:35 UTC (permalink / raw)
  To: u-boot

On Fri, Mar 09, 2018 at 12:12:59PM +0000, Alex Kiernan wrote:

> Pass tools/env/fw_env.c through indent to correct style violations. This
> commit consists of only one non-whitespace change:
> 
>   tools/env/fw_env.c:549: error: do not use assignment in if condition
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180319/e404ac1f/attachment.sig>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v2, 2/4] tools: env: Fix CamelCasing style violation
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 2/4] tools: env: Fix CamelCasing style violation Alex Kiernan
@ 2018-03-19 22:35   ` Tom Rini
  0 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2018-03-19 22:35 UTC (permalink / raw)
  To: u-boot

On Fri, Mar 09, 2018 at 12:13:00PM +0000, Alex Kiernan wrote:

> Replace HaveRedundEnv with have_redund_env to fix style violation.
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180319/23d366da/attachment.sig>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v2, 3/4] tools: env: Refactor write path of flash_io()
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 3/4] tools: env: Refactor write path of flash_io() Alex Kiernan
@ 2018-03-19 22:35   ` Tom Rini
  0 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2018-03-19 22:35 UTC (permalink / raw)
  To: u-boot

On Fri, Mar 09, 2018 at 12:13:01PM +0000, Alex Kiernan wrote:

> Extract write path of flash_io() into a separate function. This patch
> should be a functional no-op.
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> Reviewed-by: Stefano Babic <sbabic@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180319/69645ad7/attachment.sig>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [U-Boot, v2, 4/4] tools: env: Implement atomic replace for filesystem
  2018-03-09 12:13 ` [U-Boot] [PATCH v2 4/4] tools: env: Implement atomic replace for filesystem Alex Kiernan
@ 2018-03-19 22:35   ` Tom Rini
  0 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2018-03-19 22:35 UTC (permalink / raw)
  To: u-boot

On Fri, Mar 09, 2018 at 12:13:02PM +0000, Alex Kiernan wrote:

> If the U-Boot environment is stored in a regular file and redundant
> operation isn't set, then write to a temporary file and perform an
> atomic rename.
> 
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180319/63d6af61/attachment.sig>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-03-19 22:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 12:12 [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Alex Kiernan
2018-03-09 12:12 ` [U-Boot] [PATCH v2 1/4] tools: env: Pass through indent Alex Kiernan
2018-03-19 22:35   ` [U-Boot] [U-Boot,v2,1/4] " Tom Rini
2018-03-09 12:13 ` [U-Boot] [PATCH v2 2/4] tools: env: Fix CamelCasing style violation Alex Kiernan
2018-03-19 22:35   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-03-09 12:13 ` [U-Boot] [PATCH v2 3/4] tools: env: Refactor write path of flash_io() Alex Kiernan
2018-03-19 22:35   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-03-09 12:13 ` [U-Boot] [PATCH v2 4/4] tools: env: Implement atomic replace for filesystem Alex Kiernan
2018-03-19 22:35   ` [U-Boot] [U-Boot, v2, " Tom Rini
2018-03-09 12:30 ` [U-Boot] [PATCH v2 0/4] Add atomic write to fw_setenv for environments on filesystems Wolfgang Denk
2018-03-09 12:51   ` Alex Kiernan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.