This patch adds some missing unlink(2) error checks.

In cases where we are unlinking a file while already unwinding from
an error condition, we cannot return unlink errors, so those unlink
calls are left as is.
There is also code in buf.c that ignores errors, but this file was
inherited from OpenCVS and should probably be fixed there first,
if needed.

An important fix in here is where we open a new temp file with
mkstemp() and then fail to unlink it. This error is not reported up,
which is bad. No caller is deleting these files manually, and if
there is e.g. an I/O failure which prevents deletion of the file
and we still manage to write data to that file, the file might be
left behind on disk even though we want it deleted.

This is a big diff, my local branch splits it into a couple of commits:

 check for unlink(2) errors with the == -1 idiom, rather than != 0
 
 check for unlink(2) errors in got_opentempfd()
 
 check for unlink(2) errors and don't try to unlink an already renamed file
 
 introduce got_path_move_file(); based on code from lib/patch.c
 
 handle unlink(2) errors for mergepath, tmppath, and apath in apply_patch()

ok?

diff refs/heads/main refs/heads/unlink-errors
commit - 9cda65e55e3f6d565e83377712e1b81177639cda
commit + 9536370f6b2ee32526d8d5c398247f0d1c2210ca
blob - a223de14baf9eab1c747904cbc10cbaf30003d8c
blob + 7cb114b362ef4f4a6d3f385fb37066d5a97691bd
--- include/got_path.h
+++ include/got_path.h
@@ -132,3 +132,10 @@ const struct got_error *got_path_create_file(const cha
 
 /* Create a new file at a specified path, with optional content. */
 const struct got_error *got_path_create_file(const char *, const char *);
+
+/*
+ * Attempt to move an existing file to a new path, creating missing parent
+ * directories at the destination path if necessary.
+ * (Cross-mount-point moves are not yet implemented.)
+ */
+const struct got_error *got_path_move_file(const char *, const char *);
blob - 4c4e3bbe901a58b588c927fbd65dd7620b43e919
blob + 3c1f5eebe793a2f895a8529b13cdad48e3f9ccb0
--- lib/opentemp.c
+++ lib/opentemp.c
@@ -34,8 +34,12 @@ got_opentempfd(void)
 		return -1;
 
 	fd = mkstemp(name);
-	if (fd != -1)
-		unlink(name);
+	if (fd != -1) {
+		if (unlink(name) == -1) {
+			close(fd);
+			return -1;
+		}
+	}
 	return fd;
 }
 
blob - f8e3aaf634fc474c5546bddf69cefd58eb36f969
blob + 14024d2a98a433e4feb490708b08e72e42f96f75
--- lib/patch.c
+++ lib/patch.c
@@ -789,11 +789,10 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 	int do_merge = 0, file_renamed = 0;
 	char *oldlabel = NULL, *newlabel = NULL, *anclabel = NULL;
 	char *oldpath = NULL, *newpath = NULL;
-	char *tmppath = NULL, *template = NULL, *parent = NULL;
+	char *tmppath = NULL, *template = NULL;
 	char *apath = NULL, *mergepath = NULL;
 	FILE *oldfile = NULL, *tmpfile = NULL, *afile = NULL, *mergefile = NULL;
 	int outfd;
-	const char *outpath;
 	mode_t mode = GOT_DEFAULT_FILE_MODE;
 
 	*overlapcnt = 0;
@@ -842,7 +841,6 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 	err = got_opentemp_named(&tmppath, &tmpfile, template);
 	if (err)
 		goto done;
-	outpath = tmppath;
 	outfd = fileno(tmpfile);
 	err = patch_file(p, afile != NULL ? afile : oldfile, tmpfile);
 	if (err)
@@ -900,7 +898,6 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 		err = got_opentemp_named(&mergepath, &mergefile, template);
 		if (err)
 			goto done;
-		outpath = mergepath;
 		outfd = fileno(mergefile);
 
 		err = got_merge_diff3(overlapcnt, outfd, tmpfile, afile,
@@ -924,24 +921,18 @@ apply_patch(int *overlapcnt, struct got_worktree *work
 		goto done;
 	}
 
-	if (rename(outpath, newpath) == -1) {
-		if (errno != ENOENT) {
-			err = got_error_from_errno3("rename", outpath,
-			    newpath);
+	if (mergepath) {
+		err = got_path_move_file(mergepath, newpath);
+		if (err)
 			goto done;
-		}
-
-		err = got_path_dirname(&parent, newpath);
-		if (err != NULL)
+		free(mergepath);
+		mergepath = NULL;
+	} else {
+		err = got_path_move_file(tmppath, newpath);
+		if (err)
 			goto done;
-		err = got_path_mkdir(parent);
-		if (err != NULL)
-			goto done;
-		if (rename(outpath, newpath) == -1) {
-			err = got_error_from_errno3("rename", outpath,
-			    newpath);
-			goto done;
-		}
+		free(tmppath);
+		tmppath = NULL;
 	}
 
 	if (file_renamed) {
@@ -966,11 +957,10 @@ done:
 		err = report_progress(pa, old, new, GOT_STATUS_MODIFY, NULL);
 
 done:
-	free(parent);
 	free(template);
 
-	if (tmppath != NULL)
-		unlink(tmppath);
+	if (tmppath != NULL && unlink(tmppath) == -1 && err == NULL)
+		err = got_error_from_errno("unlink");
 	if (tmpfile != NULL && fclose(tmpfile) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
 	free(tmppath);
@@ -979,14 +969,14 @@ done:
 	if (oldfile != NULL && fclose(oldfile) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
 
-	if (apath != NULL)
-		unlink(apath);
+	if (apath != NULL && unlink(apath) == -1 && err == NULL)
+		err = got_error_from_errno("unlink");
 	if (afile != NULL && fclose(afile) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
 	free(apath);
 
-	if (mergepath != NULL)
-		unlink(mergepath);
+	if (mergepath != NULL && unlink(mergepath) == -1 && err == NULL)
+		err = got_error_from_errno("unlink");
 	if (mergefile != NULL && fclose(mergefile) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
 	free(mergepath);
blob - e3d7799acfdd15ffc55d19126b7c7e27a696bee6
blob + 890ae033826305afdd0fc0401d8ff424ea07eaf6
--- lib/path.c
+++ lib/path.c
@@ -537,3 +537,24 @@ done:
 		err = got_error_from_errno("close");
 	return err;
 }
+
+const struct got_error *
+got_path_move_file(const char *oldpath, const char *newpath)
+{
+	const struct got_error *err;
+
+	if (rename(oldpath, newpath) != -1)
+		return NULL;
+
+	if (errno != ENOENT)
+		return got_error_from_errno3("rename", oldpath, newpath);
+
+	err = make_parent_dirs(newpath);
+	if (err)
+		return err;
+
+	if (rename(oldpath, newpath) == -1)
+		return got_error_from_errno3("rename", oldpath, newpath);
+
+	return NULL;
+}
blob - ec8a575ccccc0191c9140e4914ca6dd642338b9c
blob + d6f94939b844a5e06b4b92355a08680544dba996
--- lib/reference.c
+++ lib/reference.c
@@ -1320,7 +1320,7 @@ done:
 	free(path_refs);
 	free(path);
 	if (tmppath) {
-		if (unlink(tmppath) != 0 && err == NULL)
+		if (unlink(tmppath) == -1 && err == NULL)
 			err = got_error_from_errno2("unlink", tmppath);
 		free(tmppath);
 	}
@@ -1454,6 +1454,8 @@ delete_packed_ref(struct got_reference *delref, struct
 			    packed_refs_path);
 			goto done;
 		}
+		free(tmppath);
+		tmppath = NULL;
 	}
 done:
 	if (delref->lf == NULL && lf)
@@ -1462,11 +1464,10 @@ done:
 		if (fclose(f) == EOF && err == NULL)
 			err = got_error_from_errno("fclose");
 	}
-	if (tmpf) {
-		unlink(tmppath);
-		if (fclose(tmpf) == EOF && err == NULL)
-			err = got_error_from_errno("fclose");
-	}
+	if (tmppath && unlink(tmppath) == -1 && err == NULL)
+		err = got_error_from_errno2("unlink", tmppath);
+	if (tmpf && fclose(tmpf) == EOF && err == NULL)
+		err = got_error_from_errno("fclose");
 	free(tmppath);
 	free(packed_refs_path);
 	free(line);
@@ -1501,7 +1502,7 @@ delete_loose_ref(struct got_reference *ref, struct got
 
 	/* XXX: check if old content matches our expectations? */
 
-	if (unlink(path) != 0)
+	if (unlink(path) == -1)
 		err = got_error_from_errno2("unlink", path);
 done:
 	if (ref->lf == NULL && lf)
blob - 30e991da06f030404cbce64de63293d385ff1ac8
blob + d0e5af7c33c6ca8a4ba2e9dc621a494ed26dda57
--- lib/worktree.c
+++ lib/worktree.c
@@ -4135,12 +4135,12 @@ schedule_for_deletion(void *arg, unsigned char status,
 		size_t root_len;
 
 		if (dirfd != -1) {
-			if (unlinkat(dirfd, de_name, 0) != 0) {
+			if (unlinkat(dirfd, de_name, 0) == -1) {
 				err = got_error_from_errno2("unlinkat",
 				    ondisk_path);
 				goto done;
 			}
-		} else if (unlink(ondisk_path) != 0) {
+		} else if (unlink(ondisk_path) == -1) {
 			err = got_error_from_errno2("unlink", ondisk_path);
 			goto done;
 		}