All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/6] reftable/stack: don't call fsync(3p) unless provided
Date: Tue, 31 Mar 2026 11:09:31 -0700	[thread overview]
Message-ID: <xmqq5x6b27ms.fsf@gitster.g> (raw)
In-Reply-To: <20260331-pks-reftable-portability-fixes-v1-2-46bfae55c68c@pks.im> (Patrick Steinhardt's message of "Tue, 31 Mar 2026 13:26:48 +0200")

Patrick Steinhardt <ps@pks.im> writes:

> Users of the reftable library are expected to provide their own function
> callback in cases they want to sync(3p) data to disk via the reftable
> write options. But if no such function was provided we end up calling
> fsync(3p) directly, which may not even be available on some systems.
>
> Drop the call to fsync(3p) and rely on the callback function
> exclusively.

Hmph, reftable-backend.c seems to do

	refs->write_options.fsync = reftable_be_fsync;

in its _be_init(), so this change is a no-op in the context of our
system, so this may be _safe_, but for a caller that wanted a fsync
to happen, returning to it without doing anything may be a bit
unexpected.  I am wondering if it should be more like

	if (!opts->fsync)
		/* BUG("whoa where is your fsync callback???") */
		reutrn -1;
	return opts->fsync(fd);

instead.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  reftable/stack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/reftable/stack.c b/reftable/stack.c
> index 1c9f21dfe1..f9ae832e3a 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -33,7 +33,7 @@ static int stack_fsync(const struct reftable_write_options *opts, int fd)
>  {
>  	if (opts->fsync)
>  		return opts->fsync(fd);
> -	return fsync(fd);
> +	return 0;
>  }
>  
>  static ssize_t reftable_write_data(int fd, const void *data, size_t size)

  reply	other threads:[~2026-03-31 18:09 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 11:26 [PATCH 0/6] reftable: some more portability improvements Patrick Steinhardt
2026-03-31 11:26 ` [PATCH 1/6] reftable/system: provide `REFTABLE_INLINE()` macro Patrick Steinhardt
2026-03-31 18:03   ` Junio C Hamano
2026-03-31 21:12   ` René Scharfe
2026-03-31 21:23     ` Junio C Hamano
2026-03-31 22:15       ` René Scharfe
2026-03-31 22:08   ` brian m. carlson
2026-04-01 12:13     ` Patrick Steinhardt
2026-03-31 11:26 ` [PATCH 2/6] reftable/stack: don't call fsync(3p) unless provided Patrick Steinhardt
2026-03-31 18:09   ` Junio C Hamano [this message]
2026-04-01 12:13     ` Patrick Steinhardt
2026-03-31 11:26 ` [PATCH 3/6] reftable/fsck: use REFTABLE_UNUSED instead of UNUSED Patrick Steinhardt
2026-03-31 18:12   ` Junio C Hamano
2026-03-31 11:26 ` [PATCH 4/6] reftable/system: add abstraction to retrieve time in milliseconds Patrick Steinhardt
2026-03-31 11:26 ` [PATCH 5/6] reftable/system: add abstraction to mmap files Patrick Steinhardt
2026-03-31 11:26 ` [PATCH 6/6] reftable: introduce "reftable-system.h" header Patrick Steinhardt
2026-03-31 18:26   ` Junio C Hamano
2026-04-01 12:14     ` Patrick Steinhardt
2026-04-01 16:37       ` Junio C Hamano
2026-04-02  6:16         ` Patrick Steinhardt
2026-04-02  7:31 ` [PATCH v2 0/5] reftable: some more portability improvements Patrick Steinhardt
2026-04-02  7:31   ` [PATCH v2 1/5] reftable: introduce "reftable-system.h" header Patrick Steinhardt
2026-04-02  7:31   ` [PATCH v2 2/5] reftable/stack: provide fsync(3p) via system header Patrick Steinhardt
2026-04-02 18:03     ` Junio C Hamano
2026-04-02  7:31   ` [PATCH v2 3/5] reftable/fsck: use REFTABLE_UNUSED instead of UNUSED Patrick Steinhardt
2026-04-02  7:31   ` [PATCH v2 4/5] reftable/system: add abstraction to retrieve time in milliseconds Patrick Steinhardt
2026-04-02 18:27     ` Junio C Hamano
2026-04-02  7:31   ` [PATCH v2 5/5] reftable/system: add abstraction to mmap files Patrick Steinhardt

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=xmqq5x6b27ms.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.