Re: [PATCH 02/10] struct ref_transaction: add a place for backends to store data

[Jeff King <peff@peff.net>]

On Tue, Aug 29, 2017 at 10:20:26AM +0200, Michael Haggerty wrote:

> `packed_ref_store` is going to want to store some transaction-wide
> data, so make a place for it.

That makes sense, although...

> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index b02dc5a7e3..d7d344de73 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -242,6 +242,7 @@ struct ref_transaction {
>  	size_t alloc;
>  	size_t nr;
>  	enum ref_transaction_state state;
> +	void *backend_data;
>  };

This is just one pointer. Once we start layering ref backends (and
already we're moving towards a "files" layer which sits atop loose and
packed backends, right?), how do we avoid backends stomping on each
other (or worse, dereferencing somebody else's data as their own
struct)?

I don't know that we necessarily need to answer that question right now,
but I'm worried that this pattern might need adjustment eventually.

I guess the hand-wavy answer is that whatever is doing the layering
would need to manage the pointers. So if you imagine that we had a
"union" backend that took two other arbitrary backends, it would
probably have something like:

  struct union_backend_data {
	void *data_a;
	void *data_b;
  }

and when it forwarded calls to the separate backends, it would give them
a view of the transaction with only their data. Something like:

  void union_backend_foo(void *be, struct ref_transaction *transaction)
  {
	struct union_backend *me = be;
	struct union_backend_data *my_data = transaction->backend_data;

        /* "a" sees only it's data, and we remember any modifications */
	transaction->backend_data = my_data->data_a;
	me->backend_a->foo(me->backend_a, transaction);
	my_data->data_a = transaction->backend_data;

        /* ditto for "b" */
	transaction->backend_data = my_data->data_b;
	me->backend_b->foo(me->backend_b, transaction);
	my_data->data_b = transaction->backend_data;

	/* and then we restore our own view */
	transaction->backend_data = my_data;
  }

-Peff