How difficult is it to move a file atomically?Richard Mawhttp://yakking.branchable.com/posts/moving-files-8-atomicity/yakkingikiwiki2016-11-28T23:13:54ZBugs in open_tmpfile()http://yakking.branchable.com/posts/moving-files-8-atomicity/comment_1_ec5c063c4b8b5ef386e7c26e05149e39/womble2 [livejournal.com]2016-11-17T02:45:18Z2016-11-17T02:45:18Z
<p>Firstly, it isn't const-correct:</p>
<pre><code>test.c: In function ‘open_tmpfile’:
test.c:12:31: warning: passing argument 1 of ‘__xpg_basename’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
strcat(template, basename(target));
^~~~~~
In file included from test.c:1:0:
/usr/include/libgen.h:34:14: note: expected ‘char *’ but argument is of type ‘const char *’
extern char *__xpg_basename (char *__path) __THROW;
^~~~~~~~~~~~~~
</code></pre>
<p>Second, <code>strcpy(template, dirname(template));</code> has undefined behaviour since <code>dirname(template)</code> will probably point to <code>template</code> and <code>strcpy()</code> does not work with overlapping buffers. I think you have to use <code>memmove()</code> or two separate buffers.</p>
<p>Finally, it leaks the template string if <code>mkstemp()</code> fails, though that's not very likely.</p>
Re: Bugs in open_tmpfile()http://yakking.branchable.com/posts/moving-files-8-atomicity/comment_2_381192724912dd814a2f5ce7daba74b1/Richard Maw2016-11-28T23:13:54Z2016-11-28T23:13:53Z
<p>Sorry I didn't notice your reply sooner.</p>
<blockquote><p> Firstly, it isn't const-correct:</p>
<pre><code> test.c: In function ‘open_tmpfile’:
test.c:12:31: warning: passing argument 1 of ‘__xpg_basename’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
strcat(template, basename(target));
^~~~~~
In file included from test.c:1:0:
/usr/include/libgen.h:34:14: note: expected ‘char *’ but argument is of type ‘const char *’
extern char *__xpg_basename (char *__path) __THROW;
^~~~~~~~~~~~~~
</code></pre></blockquote>
<p>This is from the awkwardness of there being two different definitions of basename.
I don't get this error in the full version of the code (<a href="http://yakking.branchable.com/posts/moving-files-8-atomicity/my-mv.c">http://yakking.branchable.com/posts/moving-files-8-atomicity/my-mv.c</a>)
since I need to pull in the definitions with:</p>
<pre><code>#include <libgen.h> /* dirname */
#undef basename
#include <string.h> /* basename */
</code></pre>
<p>If you are aware of a better way to pull in the definitions please enlighten me, I'm working from the manpages as to how to pull the definitions in.</p>
<blockquote><p> Second, <code>strcpy(template, dirname(template));</code> has undefined behaviour since <code>dirname(template)</code> will probably point to template and <code>strcpy()</code> does not work with overlapping buffers. I think you have to use <code>memmove()</code> or two separate buffers.</p></blockquote>
<p>Thanks, I apparently missed the warning about overlaps when I checked the documentation.
Given I only actually want to do any copy when <code>dirname(template)</code> doesn't point to <code>template</code> I've gone with:</p>
<pre><code>char *dir = NULL;
strcpy(template, target);
dir = dirname(template);
if (dir != template)
strcpy(template, dir);
</code></pre>
<blockquote><p> Finally, it leaks the template string if <code>mkstemp()</code> fails, though that's not very likely.</p></blockquote>
<p>Thanks. I have no excuse for how I missed that, I was probably writing late into the night and then didn't go back and check.</p>