# [PATCH] object-file: use `container_of()` to convert from base types 8 messages from 2026-02-18 to 2026-02-22. Participants: Justin Tobler, Patrick Steinhardt, Toon Claes, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/65017 ## Justin Tobler, 2026-02-18 21:01 Subject: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: <20260218210120.1146078-1-jltobler@gmail.com> URL: https://gitlist.dev/e/20260218210120.1146078-1-jltobler%40gmail.com ``` 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 --- 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 ``` ## Patrick Steinhardt, 2026-02-19 13:43 Subject: Re: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: URL: https://gitlist.dev/e/aZcTcgKDg6N4QW3j%40pks.im In-Reply-To: <20260218210120.1146078-1-jltobler@gmail.com> ``` 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, 2026-02-20 16:07 Subject: Re: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: <87zf53e6uh.fsf@iotcl.com> URL: https://gitlist.dev/e/87zf53e6uh.fsf%40iotcl.com In-Reply-To: <20260218210120.1146078-1-jltobler@gmail.com> ``` Justin Tobler 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 > --- > > 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, 2026-02-22 07:07 Subject: Re: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: URL: https://gitlist.dev/e/xmqqms11qmsj.fsf%40gitster.g In-Reply-To: <20260218210120.1146078-1-jltobler@gmail.com> ``` Justin Tobler 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 --- 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 ``` ## Jeff King, 2026-02-22 09:41 Subject: Re: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: <20260222094158.GA1319383@coredump.intra.peff.net> URL: https://gitlist.dev/e/20260222094158.GA1319383%40coredump.intra.peff.net In-Reply-To: ``` 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 ``` ## Justin Tobler, 2026-02-22 17:19 Subject: Re: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: URL: https://gitlist.dev/e/aZs6OvvBB4WPNx8j%40denethor In-Reply-To: <20260222094158.GA1319383@coredump.intra.peff.net> ``` 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 ``` ## Junio C Hamano, 2026-02-22 20:16 Subject: [PATCH] object-file.c: avoid container_of() of a NULL container Message-ID: URL: https://gitlist.dev/e/xmqqh5r8r0to.fsf_-_%40gitster.g In-Reply-To: ``` 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 Signed-off-by: Junio C Hamano --- 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 ``` ## Junio C Hamano, 2026-02-22 20:36 Subject: Re: [PATCH] object-file: use `container_of()` to convert from base types Message-ID: URL: https://gitlist.dev/e/xmqq8qckqzw3.fsf%40gitster.g In-Reply-To: <20260222094158.GA1319383@coredump.intra.peff.net> ``` Jeff King 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. ```