threads / patch / 65198

patchsubmodule--helper: replace malloc with xmalloc

Subject: [PATCH] submodule--helper: replace malloc with xmalloc

## tl;dr

5 messages between Mar 10, 2026 and Mar 10, 2026. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Siddharth Shrimali· Mar 10, 2026, 12:10 UTC · lore

The submodule_summary_callback() function currently uses a raw malloc() which could lead to NULL pointer dereference.

Standardize this by replacing malloc() with xmalloc() for error handling. Also, remove the unnecessary type cast and use sizeof(*temp) instead of struct name in xmalloc to improve maintainability of the code.

Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to builtin/submodule--helper.c +1 −1

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 143f7cb3cc..f3e132888f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,
 
 		if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))
 			continue;
-		temp = (struct module_cb*)malloc(sizeof(struct module_cb));
+		temp = xmalloc(sizeof(*temp));
 		temp->mod_src = p->one->mode;
 		temp->mod_dst = p->two->mode;
 		temp->oid_src = p->one->oid;

-- 
2.51.2

Patrick Steinhardt· Mar 10, 2026, 12:22 UTC · re: Siddharth Shrimali · lore

Re: [PATCH] submodule--helper: replace malloc with xmalloc

On Tue, Mar 10, 2026 at 05:40:13PM +0530, Siddharth Shrimali wrote:

Show 25 quoted lines

> The submodule_summary_callback() function currently uses a raw malloc()
> which could lead to NULL pointer dereference.
> 
> Standardize this by replacing malloc() with xmalloc() for error handling.
> Also, remove the unnecessary type cast and use sizeof(*temp) instead of
> struct name in xmalloc to improve maintainability of the code.
> 
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>  builtin/submodule--helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 143f7cb3cc..f3e132888f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,
>  
>  		if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))
>  			continue;
> -		temp = (struct module_cb*)malloc(sizeof(struct module_cb));
> +		temp = xmalloc(sizeof(*temp));
>  		temp->mod_src = p->one->mode;
>  		temp->mod_dst = p->two->mode;
>  		temp->oid_src = p->one->oid;

Yup, looks good to me, and all the other while-at-it changes are sensible, as well. Thanks!

Patrick

Junio C Hamano· Mar 10, 2026, 16:03 UTC · re: Siddharth Shrimali · lore

Re: [PATCH] submodule--helper: replace malloc with xmalloc

Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:

Show 6 quoted lines

> The submodule_summary_callback() function currently uses a raw malloc()
> which could lead to NULL pointer dereference.
>
> Standardize this by replacing malloc() with xmalloc() for error handling.
> Also, remove the unnecessary type cast and use sizeof(*temp) instead of
> struct name in xmalloc to improve maintainability of the code.

The proposed log message should explain why it is a good change to lose the cast.

Show 19 quoted lines

>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>  builtin/submodule--helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 143f7cb3cc..f3e132888f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,
>  
>  		if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))
>  			continue;
> -		temp = (struct module_cb*)malloc(sizeof(struct module_cb));
> +		temp = xmalloc(sizeof(*temp));
>  		temp->mod_src = p->one->mode;
>  		temp->mod_dst = p->two->mode;
>  		temp->oid_src = p->one->oid;

Siddharth Shrimali· Mar 10, 2026, 16:44 UTC · re: Junio C Hamano · lore

[PATCH v2] submodule--helper: replace malloc with xmalloc

The submodule_summary_callback() function currently uses a raw malloc() which could lead to a NULL pointer dereference.

Standardize this by replacing malloc() with xmalloc() for error handling. To improve maintainability, use sizeof(*temp) instead of the struct name.

While at it, drop the explicit type cast. In C, a void pointer (as returned by xmalloc) is automatically promoted to the destination pointer type. Removing the cast removes redundant syntax and prevents potential bugs by ensuring the allocation stays synchronized with the variable type if the declaration of 'temp' changes in the future.

Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
Changes in V2:
- Improved the commit message to explain the reasoning for removing
  the explicit type cast as requested by Junio.

 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to builtin/submodule--helper.c +1 −1

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 143f7cb3cc..f3e132888f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,
 
 		if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))
 			continue;
-		temp = (struct module_cb*)malloc(sizeof(struct module_cb));
+		temp = xmalloc(sizeof(*temp));
 		temp->mod_src = p->one->mode;
 		temp->mod_dst = p->two->mode;
 		temp->oid_src = p->one->oid;

-- 
2.51.2

Junio C Hamano· Mar 10, 2026, 19:35 UTC · re: Siddharth Shrimali · lore

Re: [PATCH v2] submodule--helper: replace malloc with xmalloc

Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:

Show 7 quoted lines

> The submodule_summary_callback() function currently uses a raw malloc()
> which could lead to a NULL pointer dereference.
>
> Standardize this by replacing malloc() with xmalloc() for error handling.
> To improve maintainability, use sizeof(*temp) instead of the struct name.
>
> While at it, ...

I think use of sizeof(*temp) and dropping of a cast from (void *) fall into the same bucket, i.e. to improve maintainability. Both are good changes.

Show 10 quoted lines

> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 143f7cb3cc..f3e132888f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1160,7 +1160,7 @@ static void submodule_summary_callback(struct diff_queue_struct *q,
>  
>  		if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))
>  			continue;
> -		temp = (struct module_cb*)malloc(sizeof(struct module_cb));
> +		temp = xmalloc(sizeof(*temp));

Looking good.

Thanks.