Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic

[Toon Claes <toon@iotcl.com>]

Justin Tobler <jltobler@gmail.com> writes:

> An ODB transaction handles how objects are stored temporarily and
> eventually committed. Due to object storage being implemented
> differently for a given ODB source, the ODB transactions must be
> implemented in a manner specific to the source the objects are being
> written to. To provide generic transactions, `struct odb_transaction` is
> updated to store a commit callback that can be configured to support a
> specific ODB source. For now `struct odb_transaction_files` is the
> only transaction type and what is always returned when starting a
> transaction.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  object-file.c | 80 ++++++++++++++++++++++++++++-----------------------
>  object-file.h |  6 ----
>  odb.c         |  5 +++-
>  odb.h         | 17 +++++++++++
>  4 files changed, 65 insertions(+), 43 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 7b34a2b274..d7e153c1b9 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -710,15 +710,17 @@ struct transaction_packfile {
>  	uint32_t nr_written;
>  };
>  
> -struct odb_transaction {
> -	struct odb_source *source;
> +struct odb_transaction_files {
> +	struct odb_transaction base;
>  
>  	struct tmp_objdir *objdir;
>  	struct transaction_packfile packfile;
>  };
>  
> -static void prepare_loose_object_transaction(struct odb_transaction *transaction)
> +static void prepare_loose_object_transaction(struct odb_transaction *base)
>  {
> +	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;

So you're assuming `struct odb_transaction` is the first field in
`struct odb_transaction_files`?

I think it would be safer to do this instead:

+	struct odb_transaction_files *transaction =
+		container_of(base, struct odb_transaction_files, base);

(this also can be applied in a few other places in this patch)

-- 
Cheers,
Toon