On 26 February 2018 at 22:29, Johannes Schindelin
<johannes.schindelin@xxxxxx> wrote:
> As pointed out in a review of the `--recreate-merges` patch series,
> `rollback_lock_file()` clobbers errno. Therefore, we have to report the
> error message that uses errno before calling said function.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> sequencer.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index e9baaf59bd9..5aa3dc3c95c 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -345,12 +345,14 @@ static int write_message(const void *buf, size_t len, const char *filename,
> if (msg_fd < 0)
> return error_errno(_("could not lock '%s'"), filename);
> if (write_in_full(msg_fd, buf, len) < 0) {
> + error_errno(_("could not write to '%s'"), filename);
> rollback_lock_file(&msg_file);
> - return error_errno(_("could not write to '%s'"), filename);
> + return -1;
> }
> if (append_eol && write(msg_fd, "\n", 1) < 0) {
> + error_errno(_("could not write eol to '%s'"), filename);
> rollback_lock_file(&msg_file);
> - return error_errno(_("could not write eol to '%s'"), filename);
> + return -1;
> }
> if (commit_lock_file(&msg_file) < 0) {
> rollback_lock_file(&msg_file);
> @@ -2106,16 +2108,17 @@ static int save_head(const char *head)
>
> fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
> if (fd < 0) {
> + error_errno(_("could not lock HEAD"));
> rollback_lock_file(&head_lock);
> - return error_errno(_("could not lock HEAD"));
> + return -1;
> }
I just noticed this when test-merging my series of lockfile-fixes to pu.
This `rollback_lock_file()` is not needed, since failure to take the
lock leaves it unlocked. If one wants to roll back the lock "for
clarity" or "just to be safe", then the same should arguably be done in
`write_message()`, just barely visible at the top of this diff.
Perhaps not worth a reroll. The conflict resolution between this and my
patch would be to take my hunk.
https://public-inbox.org/git/cover.1519763396.git.martin.agren@xxxxxxxxx/T/#t
Martin
