From: Junio C Hamano <gitster@pobox.com>
To: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] sequencer: rewrite save_head() in terms of write_message()
Date: Fri, 24 Mar 2023 08:01:12 -0700 [thread overview]
Message-ID: <xmqq1qle5h3b.fsf@gitster.g> (raw)
In-Reply-To: <20230323162235.995544-1-oswald.buddenhagen@gmx.de> (Oswald Buddenhagen's message of "Thu, 23 Mar 2023 17:22:35 +0100")
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.
> 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)
prev parent reply other threads:[~2023-03-24 15:01 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-23 16:22 [PATCH] sequencer: rewrite save_head() in terms of write_message() Oswald Buddenhagen
2023-03-24 15:01 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq1qle5h3b.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=oswald.buddenhagen@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).