From: Omar Polo <op@omarpolo.com>
Subject: simplify patch_file
To: gameoftrees@openbsd.org
Date: Thu, 16 Jun 2022 00:03:56 +0200

this is a preliminary refactor to simplifying some follow-up work (diff3
merge in got patch.)

it does two thing: do slightly more work in the nop case (got patch -n)
and moves the ownership of the FILE *orig up one level, from patch_file
to apply_patch.  it also simplifies the error handling in patch_file
since it doesn't have to track the file anymore.

the rationale for doing some more work even in the nop case is that in
the future i'll need the patched file for the diff3 merge machinery.

(the nop mode works as usual, this only makes us output a temporary file
even in that evenience.)

ok?

P.S.: i intend to commit this after the next release; sending out
earlier just to get some feedback earlier :)
 
diff ee2391b3cf8a1b5c9243008d81d8aebbba8aa691 5ff2781b932b8a20994fd0b342904adf452905b2
blob - 7167f541b49d2fddbb5d61d45e13bed060e2d33e
blob + 265446beb14fbf9c793d2c81f1c81d92f1d05fe3
--- lib/patch.c
+++ lib/patch.c
@@ -455,14 +455,12 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, int *l
 }
 
 static const struct got_error *
-patch_file(struct got_patch *p, const char *path, FILE *tmp, int nop,
-    mode_t *mode)
+patch_file(struct got_patch *p, FILE *orig, FILE *tmp, mode_t *mode)
 {
 	const struct got_error *err = NULL;
 	struct got_patch_hunk *h;
 	struct stat sb;
 	int lineno = 0;
-	FILE *orig;
 	off_t copypos, pos;
 	char *line = NULL;
 	size_t linesize = 0;
@@ -472,20 +470,12 @@ patch_file(struct got_patch *p, const char *path, FILE
 		h = STAILQ_FIRST(&p->head);
 		if (h == NULL || STAILQ_NEXT(h, entries) != NULL)
 			return got_error(GOT_ERR_PATCH_MALFORMED);
-		if (nop)
-			return NULL;
 		return apply_hunk(tmp, h, &lineno);
 	}
 
-	if ((orig = fopen(path, "r")) == NULL) {
-		err = got_error_from_errno2("fopen", path);
-		goto done;
-	}
+	if (fstat(fileno(orig), &sb) == -1)
+		return got_error_from_errno("fstat");
 
-	if (fstat(fileno(orig), &sb) == -1) {
-		err = got_error_from_errno("fstat");
-		goto done;
-	}
 	*mode = sb.st_mode;
 
 	copypos = 0;
@@ -495,11 +485,10 @@ patch_file(struct got_patch *p, const char *path, FILE
 		if (err != NULL && err->code == GOT_ERR_HUNK_FAILED)
 			h->err = err;
 		if (err != NULL)
-			goto done;
-		if (!nop)
-			err = copy(tmp, orig, copypos, pos);
+			return err;
+		err = copy(tmp, orig, copypos, pos);
 		if (err != NULL)
-			goto done;
+			return err;
 		copypos = pos;
 
 		err = test_hunk(orig, h);
@@ -508,46 +497,36 @@ patch_file(struct got_patch *p, const char *path, FILE
 			 * try to apply the hunk again starting the search
 			 * after the previous partial match.
 			 */
-			if (fseeko(orig, pos, SEEK_SET) == -1) {
-				err = got_error_from_errno("fseeko");
-				goto done;
-			}
+			if (fseeko(orig, pos, SEEK_SET) == -1)
+				return got_error_from_errno("fseeko");
 			linelen = getline(&line, &linesize, orig);
-			if (linelen == -1) {
-				err = got_error_from_errno("getline");
-				goto done;
-			}
+			if (linelen == -1)
+				return got_error_from_errno("getline");
 			lineno++;
 			goto tryagain;
 		}
 		if (err != NULL)
-			goto done;
+			return err;
 
 		if (lineno + 1 != h->old_from)
 			h->offset = lineno + 1 - h->old_from;
 
-		if (!nop)
-			err = apply_hunk(tmp, h, &lineno);
+		err = apply_hunk(tmp, h, &lineno);
 		if (err != NULL)
-			goto done;
+			return err;
 
 		copypos = ftello(orig);
-		if (copypos == -1) {
-			err = got_error_from_errno("ftello");
-			goto done;
-		}
+		if (copypos == -1)
+			return got_error_from_errno("ftello");
 	}
 
 	if (p->new == NULL && sb.st_size != copypos) {
 		h = STAILQ_FIRST(&p->head);
 		h->err = got_error(GOT_ERR_HUNK_FAILED);
 		err = h->err;
-	} else if (!nop && !feof(orig))
+	} else if (!feof(orig))
 		err = copy(tmp, orig, copypos, -1);
 
-done:
-	if (orig != NULL && fclose(orig) == EOF && err == NULL)
-		err = got_error_from_errno("fclose");
 	return err;
 }
 
@@ -600,7 +579,7 @@ apply_patch(struct got_worktree *worktree, struct got_
 	int file_renamed = 0;
 	char *oldpath = NULL, *newpath = NULL;
 	char *tmppath = NULL, *template = NULL, *parent = NULL;
-	FILE *tmp = NULL;
+	FILE *oldfile = NULL, *tmp = NULL;
 	mode_t mode = GOT_DEFAULT_FILE_MODE;
 
 	if (asprintf(&oldpath, "%s/%s", got_worktree_get_root_path(worktree),
@@ -623,11 +602,15 @@ apply_patch(struct got_worktree *worktree, struct got_
 		goto done;
 	}
 
-	if (!nop)
-		err = got_opentemp_named(&tmppath, &tmp, template);
+	if (p->old != NULL && (oldfile = fopen(oldpath, "r")) == NULL) {
+		err = got_error_from_errno2("open", oldpath);
+		goto done;
+	}
+
+	err = got_opentemp_named(&tmppath, &tmp, template);
 	if (err)
 		goto done;
-	err = patch_file(p, oldpath, tmp, nop, &mode);
+	err = patch_file(p, oldfile, tmp, &mode);
 	if (err)
 		goto done;
 
@@ -691,6 +674,8 @@ done:
 		err = got_error_from_errno("fclose");
 	free(tmppath);
 	free(oldpath);
+	if (oldfile != NULL && fclose(oldfile) == EOF && err == NULL)
+		err = got_error_from_errno("fclose");
 	free(newpath);
 	return err;
 }