Derrick Stolee <sto...@gmail.com> writes:

> From: Derrick Stolee <dsto...@microsoft.com>
>
> If we want to use a hashfile on the temporary file for a lockfile, then
> we need finalize_hashfile() to fully write the trailing hash but also keep
> the file descriptor open.
>
> Do this by adding a new CSUM_HASH_IN_STREAM flag along with a functional
> change that checks this flag before writing the checksum to the stream.
> This differs from previous behavior since it would be written if either
> CSUM_CLOSE or CSUM_FSYNC is provided.

I'm sorry, but I don't understand from this description what this flag
does and what it is meant to do.

> Signed-off-by: Derrick Stolee <dsto...@microsoft.com>
> ---

[...]
> diff --git a/csum-file.h b/csum-file.h
> index 9ba87f0a6c..c5a2e335e7 100644
> --- a/csum-file.h
> +++ b/csum-file.h
> @@ -27,8 +27,9 @@ extern void hashfile_checkpoint(struct hashfile *, struct 
> hashfile_checkpoint *)
>  extern int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint 
> *);
>  
>  /* finalize_hashfile flags */
> -#define CSUM_CLOSE   1
> -#define CSUM_FSYNC   2
> +#define CSUM_CLOSE           1
> +#define CSUM_FSYNC           2
> +#define CSUM_HASH_IN_STREAM  4

Especially that it is not commented / described here, and the name is
unsufficiently descriptive for me.

[...]
> diff --git a/csum-file.c b/csum-file.c
> index e6c95a6915..53ce37f7ca 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -61,11 +61,11 @@ int finalize_hashfile(struct hashfile *f, unsigned char 
> *result, unsigned int fl
>       the_hash_algo->final_fn(f->buffer, &f->ctx);
>       if (result)
>               hashcpy(result, f->buffer);
> -     if (flags & (CSUM_CLOSE | CSUM_FSYNC)) {
> -             /* write checksum and close fd */
> +     if (flags & CSUM_HASH_IN_STREAM)
>               flush(f, f->buffer, the_hash_algo->rawsz);

Wouldn't CSUM_FLUSH be a better name for this flag?

> -             if (flags & CSUM_FSYNC)
> -                     fsync_or_die(f->fd, f->name);
> +     if (flags & CSUM_FSYNC)
> +             fsync_or_die(f->fd, f->name);
> +     if (flags & CSUM_CLOSE) {
>               if (close(f->fd))
>                       die_errno("%s: sha1 file error on close", f->name);
>               fd = 0;

--
Jakub Narębski

Reply via email to