[U-Boot] [PATCH 2/2] tools: env: Implement atomic replace for filesystem

[Alex Kiernan]

On Thu, Mar 8, 2018 at 5:04 PM, Stefano Babic <sbabic at 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.

>> Signed-off-by: Alex Kiernan <alex.kiernan at 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.

-- 
Alex Kiernan