threads / patch / 65017

patchobject-file: use `container_of()` to convert from base types

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

## tl;dr

8 messages between Feb 18, 2026 and Feb 22, 2026. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Justin Tobler· Feb 18, 2026, 21:01 UTC · lore

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(-)
Show changes to object-file.c +16 −7
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
Patrick Steinhardt· Feb 19, 2026, 13:43 UTC · re: Justin Tobler · lore

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

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
Toon Claes· Feb 20, 2026, 16:07 UTC · re: Justin Tobler · lore

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

Justin Tobler <jltobler@gmail.com> writes:
Show 16 quoted lines
> 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
Junio C Hamano· Feb 22, 2026, 07:07 UTC · re: Justin Tobler · lore

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

Justin Tobler <jltobler@gmail.com> writes:
Show 8 quoted lines
>  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(-)
Show changes to diff +10 −4
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
Jeff King· Feb 22, 2026, 09:41 UTC · re: Junio C Hamano · lore

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

On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:
Show 14 quoted lines
> 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
Justin Tobler· Feb 22, 2026, 17:19 UTC · re: Jeff King · lore

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

On 26/02/22 04:41AM, Jeff King wrote:
Show 19 quoted lines
> 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

Junio C Hamano· Feb 22, 2026, 20:36 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 19 quoted lines
> 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.
Junio C Hamano· Feb 22, 2026, 20:16 UTC · re: Junio C Hamano · lore

[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., 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(-)
Show changes to object-file.c +2 −2
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

← back to recent threads