{"thread":{"id":"65198","subject":"[PATCH] submodule--helper: replace malloc with xmalloc","startedAt":"2026-03-10T12:10:21Z","lastAt":"2026-03-10T19:35:08Z","messageCount":5,"participants":["Siddharth Shrimali","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"538404","messageId":"20260310121013.39291-1-r.siddharth.shrimali@gmail.com","threadId":"65198","inReplyTo":null,"subject":"[PATCH] submodule--helper: replace malloc with xmalloc","fromName":"Siddharth Shrimali","fromEmail":"r.siddharth.shrimali@gmail.com","sentAt":"2026-03-10T12:10:13Z","receivedAt":"2026-03-10T12:10:21Z","isPatch":true,"sender":{"key":"r.siddharth.shrimali@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183274193?v=4"},"body":"The submodule_summary_callback() function currently uses a raw malloc()\nwhich could lead to NULL pointer dereference.\n\nStandardize this by replacing malloc() with xmalloc() for error handling.\nAlso, remove the unnecessary type cast and use sizeof(*temp) instead of\nstruct name in xmalloc to improve maintainability of the code.\n\nSigned-off-by: Siddharth Shrimali \n---\n builtin/submodule--helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 143f7cb3cc..f3e132888f 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n \n \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n-\t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n+\t\ttemp = xmalloc(sizeof(*temp));\n \t\ttemp->mod_src = p->one->mode;\n \t\ttemp->mod_dst = p->two->mode;\n \t\ttemp->oid_src = p->one->oid;\n-- \n2.51.2\n\n"},{"id":"538408","messageId":"abAM68lOLFVwmN5Y@pks.im","threadId":"65198","inReplyTo":"20260310121013.39291-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH] submodule--helper: replace malloc with xmalloc","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-10T12:22:03Z","receivedAt":"2026-03-10T12:22:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 10, 2026 at 05:40:13PM +0530, Siddharth Shrimali wrote:\n> The submodule_summary_callback() function currently uses a raw malloc()\n> which could lead to NULL pointer dereference.\n> \n> Standardize this by replacing malloc() with xmalloc() for error handling.\n> Also, remove the unnecessary type cast and use sizeof(*temp) instead of\n> struct name in xmalloc to improve maintainability of the code.\n> \n> Signed-off-by: Siddharth Shrimali \n> ---\n> builtin/submodule--helper.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 143f7cb3cc..f3e132888f 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n> \n> \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n> \t\t\tcontinue;\n> -\t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n> +\t\ttemp = xmalloc(sizeof(*temp));\n> \t\ttemp->mod_src = p->one->mode;\n> \t\ttemp->mod_dst = p->two->mode;\n> \t\ttemp->oid_src = p->one->oid;\n\nYup, looks good to me, and all the other while-at-it changes are\nsensible, as well. Thanks!\n\nPatrick\n"},{"id":"538469","messageId":"xmqqqzprwu1q.fsf@gitster.g","threadId":"65198","inReplyTo":"20260310121013.39291-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH] submodule--helper: replace malloc with xmalloc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-10T16:03:45Z","receivedAt":"2026-03-10T16:03:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Shrimali writes:\n\n> The submodule_summary_callback() function currently uses a raw malloc()\n> which could lead to NULL pointer dereference.\n>\n> Standardize this by replacing malloc() with xmalloc() for error handling.\n> Also, remove the unnecessary type cast and use sizeof(*temp) instead of\n> struct name in xmalloc to improve maintainability of the code.\n\nThe proposed log message should explain why it is a good change to\nlose the cast.\n\n>\n> Signed-off-by: Siddharth Shrimali \n> ---\n> builtin/submodule--helper.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 143f7cb3cc..f3e132888f 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n> \n> \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n> \t\t\tcontinue;\n> -\t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n> +\t\ttemp = xmalloc(sizeof(*temp));\n> \t\ttemp->mod_src = p->one->mode;\n> \t\ttemp->mod_dst = p->two->mode;\n> \t\ttemp->oid_src = p->one->oid;\n"},{"id":"538472","messageId":"20260310164412.47403-1-r.siddharth.shrimali@gmail.com","threadId":"65198","inReplyTo":"xmqqqzprwu1q.fsf@gitster.g","subject":"[PATCH v2] submodule--helper: replace malloc with xmalloc","fromName":"Siddharth Shrimali","fromEmail":"r.siddharth.shrimali@gmail.com","sentAt":"2026-03-10T16:44:12Z","receivedAt":"2026-03-10T16:44:21Z","isPatch":true,"sender":{"key":"r.siddharth.shrimali@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183274193?v=4"},"body":"The submodule_summary_callback() function currently uses a raw malloc()\nwhich could lead to a NULL pointer dereference.\n\nStandardize this by replacing malloc() with xmalloc() for error handling.\nTo improve maintainability, use sizeof(*temp) instead of the struct name.\n\nWhile at it, drop the explicit type cast. In C, a void pointer (as\nreturned by xmalloc) is automatically promoted to the destination\npointer type. Removing the cast removes redundant syntax and prevents\npotential bugs by ensuring the allocation stays synchronized with\nthe variable type if the declaration of 'temp' changes in the future.\n\nSigned-off-by: Siddharth Shrimali \n---\nChanges in V2:\n- Improved the commit message to explain the reasoning for removing\n the explicit type cast as requested by Junio.\n\n builtin/submodule--helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 143f7cb3cc..f3e132888f 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n \n \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n-\t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n+\t\ttemp = xmalloc(sizeof(*temp));\n \t\ttemp->mod_src = p->one->mode;\n \t\ttemp->mod_dst = p->two->mode;\n \t\ttemp->oid_src = p->one->oid;\n-- \n2.51.2\n\n"},{"id":"538510","messageId":"xmqqo6kvtr4m.fsf@gitster.g","threadId":"65198","inReplyTo":"20260310164412.47403-1-r.siddharth.shrimali@gmail.com","subject":"Re: [PATCH v2] submodule--helper: replace malloc with xmalloc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-10T19:35:05Z","receivedAt":"2026-03-10T19:35:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Shrimali writes:\n\n> The submodule_summary_callback() function currently uses a raw malloc()\n> which could lead to a NULL pointer dereference.\n>\n> Standardize this by replacing malloc() with xmalloc() for error handling.\n> To improve maintainability, use sizeof(*temp) instead of the struct name.\n>\n> While at it, ...\n\nI think use of sizeof(*temp) and dropping of a cast from (void *)\nfall into the same bucket, i.e. to improve maintainability. Both\nare good changes.\n\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 143f7cb3cc..f3e132888f 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n> \n> \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n> \t\t\tcontinue;\n> -\t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n> +\t\ttemp = xmalloc(sizeof(*temp));\n\nLooking good.\n\nThanks.\n"}]}