[PATCH] object-file: use `container_of()` to convert from base types

[PATCH] object-file: use `container_of()` to convert from base types

[Justin Tobler]

To improve code hygiene, replace direct casts from `struct
odb_transaction` and `struct odb_read_stream` to their concrete
implementations with `container_of()`.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---

Greeting,

This patch is a small cleanup following discussion in [1].

Thanks,
-Justin

[1]: <87o6m5rff8.fsf@iotcl.com>

---
 object-file.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/object-file.c b/object-file.c
index 1b62996ef0..1a24f08978 100644
--- a/object-file.c
+++ b/object-file.c
@@ -719,7 +719,8 @@ struct odb_transaction_files {
 
 static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
-	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+	struct odb_transaction_files *transaction =
+		container_of(base, struct odb_transaction_files, base);
 
 	/*
 	 * We lazily create the temporary object directory
@@ -738,7 +739,8 @@ static void prepare_loose_object_transaction(struct odb_transaction *base)
 static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
-	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+	struct odb_transaction_files *transaction =
+		container_of(base, struct odb_transaction_files, base);
 
 	/*
 	 * If we have an active ODB transaction, we issue a call that
@@ -1634,11 +1636,14 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 				 type, path, flags);
 	} else {
 		struct object_database *odb = the_repository->objects;
+		struct odb_transaction_files *files_transaction;
 		struct odb_transaction *transaction;
 
 		transaction = odb_transaction_begin(odb);
-		ret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,
-						      oid, fd,
+		files_transaction = container_of(odb->transaction,
+						 struct odb_transaction_files,
+						 base);
+		ret = index_blob_packfile_transaction(files_transaction, oid, fd,
 						      xsize_t(st->st_size),
 						      path, flags);
 		odb_transaction_commit(transaction);
@@ -1992,7 +1997,8 @@ int read_loose_object(struct repository *repo,
 
 static void odb_transaction_files_commit(struct odb_transaction *base)
 {
-	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
+	struct odb_transaction_files *transaction =
+		container_of(base, struct odb_transaction_files, base);
 
 	flush_loose_object_transaction(transaction);
 	flush_packfile_transaction(transaction);
@@ -2047,7 +2053,8 @@ struct odb_loose_read_stream {
 
 static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t sz)
 {
-	struct odb_loose_read_stream *st = (struct odb_loose_read_stream *)_st;
+	struct odb_loose_read_stream *st =
+		container_of(_st, struct odb_loose_read_stream, base);
 	size_t total_read = 0;
 
 	switch (st->z_state) {
@@ -2093,7 +2100,9 @@ static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t
 
 static int close_istream_loose(struct odb_read_stream *_st)
 {
-	struct odb_loose_read_stream *st = (struct odb_loose_read_stream *)_st;
+	struct odb_loose_read_stream *st =
+		container_of(_st, struct odb_loose_read_stream, base);
+
 	if (st->z_state == ODB_LOOSE_READ_STREAM_INUSE)
 		git_inflate_end(&st->z);
 	munmap(st->mapped, st->mapsize);

base-commit: 73fd77805fc6406f31c36212846d9e2541d19321
-- 
2.53.0

Re: [PATCH] object-file: use `container_of()` to convert from base types

[Patrick Steinhardt]

On Wed, Feb 18, 2026 at 03:01:20PM -0600, Justin Tobler wrote:
> To improve code hygiene, replace direct casts from `struct
> odb_transaction` and `struct odb_read_stream` to their concrete
> implementations with `container_of()`.

Right. The reason we want to do this is so that we become independent of
the order in which members in `struct odb_read_stream` are declared.

The changes all look good to me, thanks!

Patrick

Re: [PATCH] object-file: use `container_of()` to convert from base types

[Toon Claes]

Justin Tobler <jltobler@gmail.com> writes:

> To improve code hygiene, replace direct casts from `struct
> odb_transaction` and `struct odb_read_stream` to their concrete
> implementations with `container_of()`.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>
> Greeting,
>
> This patch is a small cleanup following discussion in [1].
>
> Thanks,
> -Justin
>
> [1]: <87o6m5rff8.fsf@iotcl.com>
>

Thanks for following up on this.

-- 
Cheers,
Toon

Re: [PATCH] object-file: use `container_of()` to convert from base types

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

>  static void prepare_loose_object_transaction(struct odb_transaction *base)
>  {
> -	struct odb_transaction_files *transaction = (struct odb_transaction_files *)base;
> +	struct odb_transaction_files *transaction =
> +		container_of(base, struct odb_transaction_files, base);
>  
>  	/*
>  	 * We lazily create the temporary object directory

This conversion triggers undefined behaviour sanitizer.  We see in
the post-context:

	if (!transaction || transaction->objdir)
		return;

which means the caller can feed NULL as base.  Taking 0 offset is
unfortunately a no-no for a NULL pointer.

Unfortunately, this patch is already part of 'next' as of 7a30cb26
(Merge branch 'jt/object-file-use-container-of' into next,
2026-02-20).

Perhaps a fix-up patch on top of the topic branch like this?

----- >8 -----
Subject: [PATCH] object-file.c: avoid container_of() of a NULL container

Even though the "struct odb_transaction" member is at the beginning
of the containing "struct odb_transaction_files", i.e., with offset 0,
using container_of() to add offset 0 to a NULL pointer would be
flagged as a bad behaviour under SANITIZE=undefined.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object-file.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git c/object-file.c w/object-file.c
index 1a24f08978..d69cb9b7e2 100644
--- c/object-file.c
+++ w/object-file.c
@@ -719,8 +719,11 @@ struct odb_transaction_files {
 
 static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
-	struct odb_transaction_files *transaction =
-		container_of(base, struct odb_transaction_files, base);
+	struct odb_transaction_files *transaction = NULL;
+
+	if (base)
+		transaction =
+			container_of(base, struct odb_transaction_files, base);
 
 	/*
 	 * We lazily create the temporary object directory
@@ -739,8 +742,11 @@ static void prepare_loose_object_transaction(struct odb_transaction *base)
 static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
-	struct odb_transaction_files *transaction =
-		container_of(base, struct odb_transaction_files, base);
+	struct odb_transaction_files *transaction = NULL;
+
+	if (base)
+		transaction =
+			container_of(base, struct odb_transaction_files, base);
 
 	/*
 	 * If we have an active ODB transaction, we issue a call that

Re: [PATCH] object-file: use `container_of()` to convert from base types

[Jeff King]

On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:

> Perhaps a fix-up patch on top of the topic branch like this?
> 
> ----- >8 -----
> Subject: [PATCH] object-file.c: avoid container_of() of a NULL container
> [...]
>  static void prepare_loose_object_transaction(struct odb_transaction *base)
>  {
> -	struct odb_transaction_files *transaction =
> -		container_of(base, struct odb_transaction_files, base);
> +	struct odb_transaction_files *transaction = NULL;
> +
> +	if (base)
> +		transaction =
> +			container_of(base, struct odb_transaction_files, base);

That works, but you can also use container_of_or_null() in the
initializer. IMHO the result is easier to read.

-Peff

Re: [PATCH] object-file: use `container_of()` to convert from base types

[Justin Tobler]

On 26/02/22 04:41AM, Jeff King wrote:
> On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:
> 
> > Perhaps a fix-up patch on top of the topic branch like this?
> > 
> > ----- >8 -----
> > Subject: [PATCH] object-file.c: avoid container_of() of a NULL container
> > [...]
> >  static void prepare_loose_object_transaction(struct odb_transaction *base)
> >  {
> > -	struct odb_transaction_files *transaction =
> > -		container_of(base, struct odb_transaction_files, base);
> > +	struct odb_transaction_files *transaction = NULL;
> > +
> > +	if (base)
> > +		transaction =
> > +			container_of(base, struct odb_transaction_files, base);
> 
> That works, but you can also use container_of_or_null() in the
> initializer. IMHO the result is easier to read.

I agree that container_of_or_null() looks a bit better here. Happy to
know about this now.

Thanks,
-Justin

[PATCH] object-file.c: avoid container_of() of a NULL container

[Junio C Hamano]

Even though the "struct odb_transaction" member is at the beginning
of the containing "struct odb_transaction_files", i.e., at offset 0,
using container_of() to add offset 0 to a NULL pointer gets flagged
as a bad behaviour under SANITIZE=undefined.

Use container_of_or_null() to work around this issue.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object-file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/object-file.c b/object-file.c
index 1a24f08978..bd580ef032 100644
--- a/object-file.c
+++ b/object-file.c
@@ -720,7 +720,7 @@ struct odb_transaction_files {
 static void prepare_loose_object_transaction(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
-		container_of(base, struct odb_transaction_files, base);
+		container_of_or_null(base, struct odb_transaction_files, base);
 
 	/*
 	 * We lazily create the temporary object directory
@@ -740,7 +740,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *base,
 					   int fd, const char *filename)
 {
 	struct odb_transaction_files *transaction =
-		container_of(base, struct odb_transaction_files, base);
+		container_of_or_null(base, struct odb_transaction_files, base);
 
 	/*
 	 * If we have an active ODB transaction, we issue a call that
-- 
2.53.0-455-gd82541b467

Re: [PATCH] object-file: use `container_of()` to convert from base types

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:
>
>> Perhaps a fix-up patch on top of the topic branch like this?
>> 
>> ----- >8 -----
>> Subject: [PATCH] object-file.c: avoid container_of() of a NULL container
>> [...]
>>  static void prepare_loose_object_transaction(struct odb_transaction *base)
>>  {
>> -	struct odb_transaction_files *transaction =
>> -		container_of(base, struct odb_transaction_files, base);
>> +	struct odb_transaction_files *transaction = NULL;
>> +
>> +	if (base)
>> +		transaction =
>> +			container_of(base, struct odb_transaction_files, base);
>
> That works, but you can also use container_of_or_null() in the
> initializer. IMHO the result is easier to read.

Thanks.  Will use that.