Re: [PATCH 1/2] sequencer: factor out rewrite_file()

On Tue, Oct 31, 2017 at 05:33:57PM +0100, Kevin Daudt wrote:

On Tue, Oct 31, 2017 at 10:54:21AM +0100, René Scharfe wrote:

quoted

Reduce code duplication by extracting a function for rewriting an
existing file.

Signed-off-by: Rene Scharfe <redacted>
---
 sequencer.c | 46 +++++++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 29 deletions(-)

diff --git a/sequencer.c b/sequencer.c > > index f2a10cc4f2..17360eb38a 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -2665,6 +2665,20 @@ int check_todo_list(void)
 	return res;
 }
 
+static int rewrite_file(const char *path, const char *buf, size_t len)
+{
+	int rc = 0;
+	int fd = open(path, O_WRONLY);
+	if (fd < 0)
+		return error_errno(_("could not open '%s' for writing"), path);
+	if (write_in_full(fd, buf, len) < 0)
+		rc = error_errno(_("could not write to '%s'"), path);
+	if (!rc && ftruncate(fd, len) < 0)
+		rc = error_errno(_("could not truncate '%s'"), path);
+	close(fd);
+	return rc;
+}
+
 /* skip picking commits whose parents are unchanged */
 int skip_unnecessary_picks(void)
 {

@@ -2737,29 +2751,11 @@ int skip_unnecessary_picks(void)
 		}
 		close(fd);
 
-		fd = open(rebase_path_todo(), O_WRONLY, 0666);
-		if (fd < 0) {
-			error_errno(_("could not open '%s' for writing"),
-				    rebase_path_todo());
+		if (rewrite_file(rebase_path_todo(), todo_list.buf.buf + offset,
+				 todo_list.buf.len - offset) < 0) {
 			todo_list_release(&todo_list);
 			return -1;
 		}
-		if (write_in_full(fd, todo_list.buf.buf + offset,
-				todo_list.buf.len - offset) < 0) {
-			error_errno(_("could not write to '%s'"),
-				    rebase_path_todo());
-			close(fd);
-			todo_list_release(&todo_list);

Is this missing on purpose in the new situation?

I wasn't looking at the context, only the changed lines. After reading
it again, it's clear that nothing is missing (the freeing of todo_list).

Kevin