On 09/03/2018 10:54, Alex Kiernan wrote:
On Thu, Mar 8, 2018 at 5:04 PM, Stefano Babic sbabic@denx.de wrote:
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:
At this point you're writing the new environment, so we should hold a lock to avoid concurrent writes.
This is already done, even if not in the way I like. tools/env/fw_env_main.c calls flock() to acquire the resource. This works using fw_printenv / fw_setenv, but not as library. Library's users as me have to provide themselves a lock before calling the fw_* functions.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com
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 ?
I went round all the temp functions before in the end deciding to fix it. However it looks like I totally misread the contract that mkstemp delivers - I'd thought it didn't pass you the name back, but it clearly does; I'll go update it.
Best regards, Stefano Babic