{"thread":{"id":"65017","subject":"[PATCH] object-file: use `container_of()` to convert from base types","startedAt":"2026-02-18T21:01:32Z","lastAt":"2026-02-22T20:36:30Z","messageCount":8,"participants":["Justin Tobler","Patrick Steinhardt","Toon Claes","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"536333","messageId":"20260218210120.1146078-1-jltobler@gmail.com","threadId":"65017","inReplyTo":null,"subject":"[PATCH] object-file: use `container_of()` to convert from base types","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-18T21:01:20Z","receivedAt":"2026-02-18T21:01:32Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"To improve code hygiene, replace direct casts from `struct\nodb_transaction` and `struct odb_read_stream` to their concrete\nimplementations with `container_of()`.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n\nGreeting,\n\nThis patch is a small cleanup following discussion in [1].\n\nThanks,\n-Justin\n\n[1]: <87o6m5rff8.fsf@iotcl.com>\n\n---\n object-file.c | 23 ++++++++++++++++-------\n 1 file changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 1b62996ef0..1a24f08978 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -719,7 +719,8 @@ struct odb_transaction_files {\n \n static void prepare_loose_object_transaction(struct odb_transaction *base)\n {\n-\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n+\tstruct odb_transaction_files *transaction =\n+\t\tcontainer_of(base, struct odb_transaction_files, base);\n \n \t/*\n \t * We lazily create the temporary object directory\n@@ -738,7 +739,8 @@ static void prepare_loose_object_transaction(struct odb_transaction *base)\n static void fsync_loose_object_transaction(struct odb_transaction *base,\n \t\t\t\t\t   int fd, const char *filename)\n {\n-\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n+\tstruct odb_transaction_files *transaction =\n+\t\tcontainer_of(base, struct odb_transaction_files, base);\n \n \t/*\n \t * If we have an active ODB transaction, we issue a call that\n@@ -1634,11 +1636,14 @@ int index_fd(struct index_state *istate, struct object_id *oid,\n \t\t\t\t type, path, flags);\n \t} else {\n \t\tstruct object_database *odb = the_repository->objects;\n+\t\tstruct odb_transaction_files *files_transaction;\n \t\tstruct odb_transaction *transaction;\n \n \t\ttransaction = odb_transaction_begin(odb);\n-\t\tret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,\n-\t\t\t\t\t\t      oid, fd,\n+\t\tfiles_transaction = container_of(odb->transaction,\n+\t\t\t\t\t\t struct odb_transaction_files,\n+\t\t\t\t\t\t base);\n+\t\tret = index_blob_packfile_transaction(files_transaction, oid, fd,\n \t\t\t\t\t\t      xsize_t(st->st_size),\n \t\t\t\t\t\t      path, flags);\n \t\todb_transaction_commit(transaction);\n@@ -1992,7 +1997,8 @@ int read_loose_object(struct repository *repo,\n \n static void odb_transaction_files_commit(struct odb_transaction *base)\n {\n-\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n+\tstruct odb_transaction_files *transaction =\n+\t\tcontainer_of(base, struct odb_transaction_files, base);\n \n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n@@ -2047,7 +2053,8 @@ struct odb_loose_read_stream {\n \n static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t sz)\n {\n-\tstruct odb_loose_read_stream *st = (struct odb_loose_read_stream *)_st;\n+\tstruct odb_loose_read_stream *st =\n+\t\tcontainer_of(_st, struct odb_loose_read_stream, base);\n \tsize_t total_read = 0;\n \n \tswitch (st->z_state) {\n@@ -2093,7 +2100,9 @@ static ssize_t read_istream_loose(struct odb_read_stream *_st, char *buf, size_t\n \n static int close_istream_loose(struct odb_read_stream *_st)\n {\n-\tstruct odb_loose_read_stream *st = (struct odb_loose_read_stream *)_st;\n+\tstruct odb_loose_read_stream *st =\n+\t\tcontainer_of(_st, struct odb_loose_read_stream, base);\n+\n \tif (st->z_state == ODB_LOOSE_READ_STREAM_INUSE)\n \t\tgit_inflate_end(&st->z);\n \tmunmap(st->mapped, st->mapsize);\n\nbase-commit: 73fd77805fc6406f31c36212846d9e2541d19321\n-- \n2.53.0\n\n"},{"id":"536404","messageId":"aZcTcgKDg6N4QW3j@pks.im","threadId":"65017","inReplyTo":"20260218210120.1146078-1-jltobler@gmail.com","subject":"Re: [PATCH] object-file: use `container_of()` to convert from base types","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-19T13:43:14Z","receivedAt":"2026-02-19T13:43:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Feb 18, 2026 at 03:01:20PM -0600, Justin Tobler wrote:\n> To improve code hygiene, replace direct casts from `struct\n> odb_transaction` and `struct odb_read_stream` to their concrete\n> implementations with `container_of()`.\n\nRight. The reason we want to do this is so that we become independent of\nthe order in which members in `struct odb_read_stream` are declared.\n\nThe changes all look good to me, thanks!\n\nPatrick\n"},{"id":"536530","messageId":"87zf53e6uh.fsf@iotcl.com","threadId":"65017","inReplyTo":"20260218210120.1146078-1-jltobler@gmail.com","subject":"Re: [PATCH] object-file: use `container_of()` to convert from base types","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-02-20T16:07:50Z","receivedAt":"2026-02-20T16:08:05Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> To improve code hygiene, replace direct casts from `struct\n> odb_transaction` and `struct odb_read_stream` to their concrete\n> implementations with `container_of()`.\n>\n> Signed-off-by: Justin Tobler <jltobler@gmail.com>\n> ---\n>\n> Greeting,\n>\n> This patch is a small cleanup following discussion in [1].\n>\n> Thanks,\n> -Justin\n>\n> [1]: <87o6m5rff8.fsf@iotcl.com>\n>\n\nThanks for following up on this.\n\n-- \nCheers,\nToon\n"},{"id":"536632","messageId":"xmqqms11qmsj.fsf@gitster.g","threadId":"65017","inReplyTo":"20260218210120.1146078-1-jltobler@gmail.com","subject":"Re: [PATCH] object-file: use `container_of()` to convert from base types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T07:07:08Z","receivedAt":"2026-02-22T07:07:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n>  static void prepare_loose_object_transaction(struct odb_transaction *base)\n>  {\n> -\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n> +\tstruct odb_transaction_files *transaction =\n> +\t\tcontainer_of(base, struct odb_transaction_files, base);\n>  \n>  \t/*\n>  \t * We lazily create the temporary object directory\n\nThis conversion triggers undefined behaviour sanitizer.  We see in\nthe post-context:\n\n\tif (!transaction || transaction->objdir)\n\t\treturn;\n\nwhich means the caller can feed NULL as base.  Taking 0 offset is\nunfortunately a no-no for a NULL pointer.\n\nUnfortunately, this patch is already part of 'next' as of 7a30cb26\n(Merge branch 'jt/object-file-use-container-of' into next,\n2026-02-20).\n\nPerhaps a fix-up patch on top of the topic branch like this?\n\n----- >8 -----\nSubject: [PATCH] object-file.c: avoid container_of() of a NULL container\n\nEven though the \"struct odb_transaction\" member is at the beginning\nof the containing \"struct odb_transaction_files\", i.e., with offset 0,\nusing container_of() to add offset 0 to a NULL pointer would be\nflagged as a bad behaviour under SANITIZE=undefined.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n object-file.c | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git c/object-file.c w/object-file.c\nindex 1a24f08978..d69cb9b7e2 100644\n--- c/object-file.c\n+++ w/object-file.c\n@@ -719,8 +719,11 @@ struct odb_transaction_files {\n \n static void prepare_loose_object_transaction(struct odb_transaction *base)\n {\n-\tstruct odb_transaction_files *transaction =\n-\t\tcontainer_of(base, struct odb_transaction_files, base);\n+\tstruct odb_transaction_files *transaction = NULL;\n+\n+\tif (base)\n+\t\ttransaction =\n+\t\t\tcontainer_of(base, struct odb_transaction_files, base);\n \n \t/*\n \t * We lazily create the temporary object directory\n@@ -739,8 +742,11 @@ static void prepare_loose_object_transaction(struct odb_transaction *base)\n static void fsync_loose_object_transaction(struct odb_transaction *base,\n \t\t\t\t\t   int fd, const char *filename)\n {\n-\tstruct odb_transaction_files *transaction =\n-\t\tcontainer_of(base, struct odb_transaction_files, base);\n+\tstruct odb_transaction_files *transaction = NULL;\n+\n+\tif (base)\n+\t\ttransaction =\n+\t\t\tcontainer_of(base, struct odb_transaction_files, base);\n \n \t/*\n \t * If we have an active ODB transaction, we issue a call that\n"},{"id":"536637","messageId":"20260222094158.GA1319383@coredump.intra.peff.net","threadId":"65017","inReplyTo":"xmqqms11qmsj.fsf@gitster.g","subject":"Re: [PATCH] object-file: use `container_of()` to convert from base types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-02-22T09:41:58Z","receivedAt":"2026-02-22T09:42:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:\n\n> Perhaps a fix-up patch on top of the topic branch like this?\n> \n> ----- >8 -----\n> Subject: [PATCH] object-file.c: avoid container_of() of a NULL container\n> [...]\n>  static void prepare_loose_object_transaction(struct odb_transaction *base)\n>  {\n> -\tstruct odb_transaction_files *transaction =\n> -\t\tcontainer_of(base, struct odb_transaction_files, base);\n> +\tstruct odb_transaction_files *transaction = NULL;\n> +\n> +\tif (base)\n> +\t\ttransaction =\n> +\t\t\tcontainer_of(base, struct odb_transaction_files, base);\n\nThat works, but you can also use container_of_or_null() in the\ninitializer. IMHO the result is easier to read.\n\n-Peff\n"},{"id":"536651","messageId":"aZs6OvvBB4WPNx8j@denethor","threadId":"65017","inReplyTo":"20260222094158.GA1319383@coredump.intra.peff.net","subject":"Re: [PATCH] object-file: use `container_of()` to convert from base types","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-22T17:19:29Z","receivedAt":"2026-02-22T17:19:31Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/22 04:41AM, Jeff King wrote:\n> On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:\n> \n> > Perhaps a fix-up patch on top of the topic branch like this?\n> > \n> > ----- >8 -----\n> > Subject: [PATCH] object-file.c: avoid container_of() of a NULL container\n> > [...]\n> >  static void prepare_loose_object_transaction(struct odb_transaction *base)\n> >  {\n> > -\tstruct odb_transaction_files *transaction =\n> > -\t\tcontainer_of(base, struct odb_transaction_files, base);\n> > +\tstruct odb_transaction_files *transaction = NULL;\n> > +\n> > +\tif (base)\n> > +\t\ttransaction =\n> > +\t\t\tcontainer_of(base, struct odb_transaction_files, base);\n> \n> That works, but you can also use container_of_or_null() in the\n> initializer. IMHO the result is easier to read.\n\nI agree that container_of_or_null() looks a bit better here. Happy to\nknow about this now.\n\nThanks,\n-Justin\n"},{"id":"536662","messageId":"xmqqh5r8r0to.fsf_-_@gitster.g","threadId":"65017","inReplyTo":"xmqqms11qmsj.fsf@gitster.g","subject":"[PATCH] object-file.c: avoid container_of() of a NULL container","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T20:16:19Z","receivedAt":"2026-02-22T20:16:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though the \"struct odb_transaction\" member is at the beginning\nof the containing \"struct odb_transaction_files\", i.e., at offset 0,\nusing container_of() to add offset 0 to a NULL pointer gets flagged\nas a bad behaviour under SANITIZE=undefined.\n\nUse container_of_or_null() to work around this issue.\n\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n object-file.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 1a24f08978..bd580ef032 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -720,7 +720,7 @@ struct odb_transaction_files {\n static void prepare_loose_object_transaction(struct odb_transaction *base)\n {\n \tstruct odb_transaction_files *transaction =\n-\t\tcontainer_of(base, struct odb_transaction_files, base);\n+\t\tcontainer_of_or_null(base, struct odb_transaction_files, base);\n \n \t/*\n \t * We lazily create the temporary object directory\n@@ -740,7 +740,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *base,\n \t\t\t\t\t   int fd, const char *filename)\n {\n \tstruct odb_transaction_files *transaction =\n-\t\tcontainer_of(base, struct odb_transaction_files, base);\n+\t\tcontainer_of_or_null(base, struct odb_transaction_files, base);\n \n \t/*\n \t * If we have an active ODB transaction, we issue a call that\n-- \n2.53.0-455-gd82541b467\n\n"},{"id":"536665","messageId":"xmqq8qckqzw3.fsf@gitster.g","threadId":"65017","inReplyTo":"20260222094158.GA1319383@coredump.intra.peff.net","subject":"Re: [PATCH] object-file: use `container_of()` to convert from base types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T20:36:28Z","receivedAt":"2026-02-22T20:36:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Feb 21, 2026 at 11:07:08PM -0800, Junio C Hamano wrote:\n>\n>> Perhaps a fix-up patch on top of the topic branch like this?\n>> \n>> ----- >8 -----\n>> Subject: [PATCH] object-file.c: avoid container_of() of a NULL container\n>> [...]\n>>  static void prepare_loose_object_transaction(struct odb_transaction *base)\n>>  {\n>> -\tstruct odb_transaction_files *transaction =\n>> -\t\tcontainer_of(base, struct odb_transaction_files, base);\n>> +\tstruct odb_transaction_files *transaction = NULL;\n>> +\n>> +\tif (base)\n>> +\t\ttransaction =\n>> +\t\t\tcontainer_of(base, struct odb_transaction_files, base);\n>\n> That works, but you can also use container_of_or_null() in the\n> initializer. IMHO the result is easier to read.\n\nThanks.  Will use that.\n"}]}