finalize_object_file(): refactor unlink_or_warn() placement

As soon as we've tried to link() a temporary object into place, we then unlink() the tempfile immediately, whether we were successful or not. For the success case, this is because we no longer need the old file (it's now linked into place). For the error case, there are two outcomes. Either we got EEXIST, in which case we consider the collision to be a noop. Or we got a system error, in which we case we are just cleaning up after ourselves. Using a single line for all of these cases has some problems: - in the error case, our unlink() may clobber errno, which we use in the error message - for the collision case, there's a FIXME that indicates we should do a collision check. In preparation for implementing that, we'll need to actually hold on to the file. Split these three cases into their own calls to unlink_or_warn(). This is more verbose, but lets us do the right thing in each case. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Sep 26, 2024 at 11:22 UTC 9ca7c2c13bc26afe673172a9b417a43519381968
1 file changed +6 -1
object-file.c
+6 -1
@@ -1911,6 +1911,8 @@ int finalize_object_file(const char *tmpfile, const char *filename)
1911 goto try_rename;
1912 else if (link(tmpfile, filename))
1913 ret = errno;
1914 + else
1915 + unlink_or_warn(tmpfile);
1916
1917 /*
1918 * Coda hack - coda doesn't like cross-directory links,
@@ -1932,12 +1934,15 @@ int finalize_object_file(const char *tmpfile, const char *filename)
1934 else
1935 ret = errno;
1936 }
1935 - unlink_or_warn(tmpfile);
1937 if (ret) {
1938 if (ret != EEXIST) {
1939 + int saved_errno = errno;
1940 + unlink_or_warn(tmpfile);
1941 + errno = saved_errno;
1942 return error_errno(_("unable to write file %s"), filename);
1943 }
1944 /* FIXME!!! Collision check here ? */
1945 + unlink_or_warn(tmpfile);
1946 }
1947
1948 out: