threads / patch / 59453

patchsequencer: rewrite save_head() in terms of write_message()

Subject: [PATCH] sequencer: rewrite save_head() in terms of write_message()

## tl;dr

2 messages between Mar 23, 2023 and Mar 24, 2023. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Oswald Buddenhagen· Mar 23, 2023, 16:22 UTC · lore
Saves some code duplication.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sequencer.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)
Show changes to sequencer.c +1 −19
diff --git a/sequencer.c b/sequencer.c
index 3be23d7ca2..ff985fb2e9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3187,25 +3187,7 @@ static int create_seq_dir(struct repository *r)
 
 static int save_head(const char *head)
 {
-	struct lock_file head_lock = LOCK_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	int fd;
-	ssize_t written;
-
-	fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
-	if (fd < 0)
-		return error_errno(_("could not lock HEAD"));
-	strbuf_addf(&buf, "%s\n", head);
-	written = write_in_full(fd, buf.buf, buf.len);
-	strbuf_release(&buf);
-	if (written < 0) {
-		error_errno(_("could not write to '%s'"), git_path_head_file());
-		rollback_lock_file(&head_lock);
-		return -1;
-	}
-	if (commit_lock_file(&head_lock) < 0)
-		return error(_("failed to finalize '%s'"), git_path_head_file());
-	return 0;
+	return write_message(head, strlen(head), git_path_head_file(), 1);
 }
 
 static int rollback_is_safe(void)
-- 
2.40.0.152.g15d061e6df
Junio C Hamano· Mar 24, 2023, 15:01 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH] sequencer: rewrite save_head() in terms of write_message()

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
> Saves some code duplication.

There are two and a half small behaviour changes, but I think it is a good change overall.

 - We used to fail with "could not lock HEAD", but now we give the
   real filename for "sequencer/head", when the locking fails.
 - We used to write the file in a single write_in_full() call, which
   in practice would have been done in a single write() system call.
   Now we do it in two steps, the body of the line in one, and a
   single terminating LF in another.  Also, there is a new error
   mode (object name gets written successfully but writing the
   terminating LF fails) with a new error message.

The former is definite improvement that may help debugging when things go wrong. The latter should not be measurable. The new error mode probably is not noticeable in practice.

Looking good.  Will queue.  Thanks.
Show 36 quoted lines
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---
>  sequencer.c | 20 +-------------------
>  1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 3be23d7ca2..ff985fb2e9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3187,25 +3187,7 @@ static int create_seq_dir(struct repository *r)
>  
>  static int save_head(const char *head)
>  {
> -	struct lock_file head_lock = LOCK_INIT;
> -	struct strbuf buf = STRBUF_INIT;
> -	int fd;
> -	ssize_t written;
> -
> -	fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
> -	if (fd < 0)
> -		return error_errno(_("could not lock HEAD"));
> -	strbuf_addf(&buf, "%s\n", head);
> -	written = write_in_full(fd, buf.buf, buf.len);
> -	strbuf_release(&buf);
> -	if (written < 0) {
> -		error_errno(_("could not write to '%s'"), git_path_head_file());
> -		rollback_lock_file(&head_lock);
> -		return -1;
> -	}
> -	if (commit_lock_file(&head_lock) < 0)
> -		return error(_("failed to finalize '%s'"), git_path_head_file());
> -	return 0;
> +	return write_message(head, strlen(head), git_path_head_file(), 1);
>  }
>  
>  static int rollback_is_safe(void)

← back to recent threads