Re: [PATCH v3 06/11] odb/transaction: propagate begin errors

[Junio C Hamano <gitster@pobox.com>]

Justin Tobler <jltobler@gmail.com> writes:

> When `odb_transaction_begin()` is invoked, the function returns the
> transaction pointer directly. There is no way for the backend to
> signal that it failed to set up its state, such as when creating the
> temporary object directory backing the transaction.
>
> In a subsequent commit, git-receive-pack(1) starts using ODB
> transactions and needs to be able to report such failures rather
> than silently ignore them. Refactor `odb_transaction_begin()` to
> return an int error code and write the resulting transaction into an
> out parameter. Also introduce `odb_transaction_begin_or_die()` as a
> convenience for callsites that do not need to handle errors
> explicitly.
>
> Note that `odb_transaction_begin()` now returns an error when the ODB
> already has an inflight transaction pending. ODB transaction call sites
> that may encounter an inflight transaction are updated to explicitly
> handle this case.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
> ...
> diff --git a/odb/transaction.c b/odb/transaction.c
> index b16e07aebf..a5fba7f908 100644
> --- a/odb/transaction.c
> +++ b/odb/transaction.c
> @@ -1,15 +1,20 @@
>  #include "git-compat-util.h"
> +#include "gettext.h"
>  #include "odb/source.h"
>  #include "odb/transaction.h"
>  
> -struct odb_transaction *odb_transaction_begin(struct object_database *odb)
> +int odb_transaction_begin(struct object_database *odb,
> +			  struct odb_transaction **out)
>  {
> +	int ret;
> +
>  	if (odb->transaction)
> -		return NULL;
> +		return error(_("object database transaction already pending"));
>  
> -	odb_source_begin_transaction(odb->sources, &odb->transaction);
> +	ret = odb_source_begin_transaction(odb->sources, out);
> +	odb->transaction = *out;

Can odb_source_begin_transaction() ever fail?  If so, and when it
fails, would *out be left untouched?  I am wondering if we want

	if (!(ret = odb_source_begin_transaction(odb->sources, out)))
        	odb->transaction = *out;

or something like that.