Hi alex, On 08/03/2018 12:52, 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. >
Even if it is not explicitely set (IMHO it should), this code can be used as library and linked to own application. That means that concurrency can happens. I mean: > Signed-off-by: Alex Kiernan <[email protected]> > --- > > tools/env/fw_env.c | 81 > ++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 78 insertions(+), 3 deletions(-) > > diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c > index 2df3504..b814c4e 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> > @@ -1229,9 +1230,46 @@ 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/uboot.tmp", *dname); Filename is fixed - should we not use mkstemp ? > + if (rc == -1) > + goto err; > + > + rc = open(temp_name, O_RDWR | O_CREAT | O_TRUNC, 0700); > + if (rc == -1) { > + /* fall back to in place write */ > + 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 (HaveRedundEnv) { > /* switch to next partition for writing */ > @@ -1247,8 +1285,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); > @@ -1260,7 +1307,7 @@ static int flash_io_write (int fd_current) > DEVNAME (dev_current), strerror (errno)); > } > > - if (HaveRedundEnv) { > + if (fd_current != fd_target) { > if (fsync(fd_target) && > !(errno == EINVAL || errno == EROFS)) { > fprintf (stderr, > @@ -1275,6 +1322,34 @@ static int flash_io_write (int fd_current) > 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; > Best regards, Stefano -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: [email protected] ===================================================================== _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
