Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD
On Wed, 26 Oct 2005, Johannes Schindelin wrote:
Show 9 quoted lines
>
> On Wed, 26 Oct 2005, Jan Harkes wrote:
>
> > - if (ret == EXDEV && !rename(tmpfile, filename))
> > + if (ret && ret != EEXIST && !rename(tmpfile, filename))
>
> We don't need to test for ret, as we're already in a "if (ret)" block.
> What would happen if ret==EEXIST, and we try to rename() nevertheless? It
> should fail, too, right?
No, the rename will succeed and overwrite the existing file.
Which is strictly correct, but it's the wrong thing if we ever want to do collission detection.
So I'd actually prefer something like this patch instead.
Linus
---
diff --git a/sha1_file.c b/sha1_file.c
index 7fdc469..642f00d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1232,19 +1232,20 @@ static int link_temp_to_file(const char
int move_temp_to_file(const char *tmpfile, char *filename)
{
int ret = link_temp_to_file(tmpfile, filename);
- if (ret) {
- /*
- * Coda hack - coda doesn't like cross-directory links,
- * so we fall back to a rename, which will mean that it
- * won't be able to check collisions, but that's not a
- * big deal.
- *
- * The same holds for FAT formatted media.
- *
- * When this succeeds, we just return 0. We have nothing
- * left to unlink.
- */
- if ((ret == EXDEV || ret == ENOTSUP) && !rename(tmpfile, filename))
+
+ /*
+ * Coda hack - coda doesn't like cross-directory links,
+ * so we fall back to a rename, which will mean that it
+ * won't be able to check collisions, but that's not a
+ * big deal.
+ *
+ * The same holds for FAT formatted media.
+ *
+ * When this succeeds, we just return 0. We have nothing
+ * left to unlink.
+ */
+ if (ret && ret != EEXIST) {
+ if (!rename(tmpfile, filename))
return 0;
ret = errno;
}