On Fri, Oct 08 2021, Taylor Blau wrote:
> When writing a MIDX, we atomically move the new MIDX into place via
> commit_lock_file(), but do not check to see if that call was successful.
>
> Make sure that we do check in order to prevent us from incorrectly
> reporting that we wrote a new MIDX if we actually encountered an error.
>
>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> midx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/midx.c b/midx.c
> index 137af3fc67..e79fb4427d 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -1434,7 +1434,8 @@ static int write_midx_internal(const char *object_dir,
> if (ctx.m)
> close_object_store(the_repository->objects);
>
> - commit_lock_file(&lk);
> + if (commit_lock_file(&lk) < 0)
> + die_errno(_("could not write multi-pack-index"));
>
> clear_midx_files_ext(object_dir, ".bitmap", midx_hash);
> clear_midx_files_ext(object_dir, ".rev", midx_hash);
As an aside I've wondered with this API whether something like this
wouldn't make sense (obviously with a better message):
diff --git a/lockfile.c b/lockfile.c
index cc9a4b84283..6632a634f00 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -175,6 +175,7 @@ int hold_lock_file_for_update_timeout_mode(struct lock_file *lk,
long timeout_ms, int mode)
{
int fd = lock_file_timeout(lk, path, flags, timeout_ms, mode);
+ lk->flags = flags;
if (fd < 0) {
if (flags & LOCK_DIE_ON_ERROR)
unable_to_lock_die(path, errno);
@@ -209,6 +210,8 @@ int commit_lock_file(struct lock_file *lk)
int save_errno = errno;
free(result_path);
errno = save_errno;
+ if (lk->flags & LOCK_DIE_ON_ERROR)
+ die_errno("noes");
return -1;
}
free(result_path);
diff --git a/lockfile.h b/lockfile.h
index db93e6ba73e..40a9d91a6c5 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -119,6 +119,7 @@
struct lock_file {
struct tempfile *tempfile;
+ int flags;
};
#define LOCK_INIT { NULL }
But a quick grep of the users reveals that some die on lock failure, but
are happy to ignore commit_lock_file() failures, or maybe those are also
bugs similar to the one you're fixing here & we could fix the API more
generally.
