[PATCH] maintenance: use xcalloc instead of xmalloc where possible

[PATCH] maintenance: use xcalloc instead of xmalloc where possible

[Rose via GitGitGadget]

From: Seija <doremylover123@gmail.com>

We can avoid having to call memset by calling xcalloc directly

Signed-off-by: Seija doremylover123@gmail.com
---
    maintenance: use xcalloc instead of xmalloc where possible
    
    We can avoid having to call memset by calling xcalloc directly
    
    Signed-off-by: Seija doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v1
Pull-Request: https://github.com/git/git/pull/1390

 remote.c    | 4 +---
 submodule.c | 3 +--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index 60869beebe7..75315f3563f 100644
--- a/remote.c
+++ b/remote.c
@@ -2741,9 +2741,7 @@ void apply_push_cas(struct push_cas_option *cas,
 
 struct remote_state *remote_state_new(void)
 {
-	struct remote_state *r = xmalloc(sizeof(*r));
-
-	memset(r, 0, sizeof(*r));
+	struct remote_state *r = xcalloc(1, sizeof(*r));
 
 	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
 	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
diff --git a/submodule.c b/submodule.c
index 8ac2fca855d..4ca4f6c6590 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1464,8 +1464,7 @@ static const struct submodule *get_non_gitmodules_submodule(const char *path)
 	if (!name)
 		return NULL;
 
-	ret = xmalloc(sizeof(*ret));
-	memset(ret, 0, sizeof(*ret));
+	ret = xcalloc(1, sizeof(*ret));
 	ret->path = name;
 	ret->name = name;
 

base-commit: 805265fcf7a737664a8321aaf4a0587b78435184
-- 
gitgitgadget

Re: [PATCH] maintenance: use xcalloc instead of xmalloc where possible

[Jeff Hostetler]

On 12/5/22 9:48 AM, Rose via GitGitGadget wrote:
> From: Seija <doremylover123@gmail.com>
> 
> We can avoid having to call memset by calling xcalloc directly
> 
> Signed-off-by: Seija doremylover123@gmail.com
> ---
>      maintenance: use xcalloc instead of xmalloc where possible
>      
>      We can avoid having to call memset by calling xcalloc directly
>      
>      Signed-off-by: Seija doremylover123@gmail.com
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v1
> Pull-Request: https://github.com/git/git/pull/1390
> 
>   remote.c    | 4 +---
>   submodule.c | 3 +--
>   2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/remote.c b/remote.c
> index 60869beebe7..75315f3563f 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2741,9 +2741,7 @@ void apply_push_cas(struct push_cas_option *cas,
>   
>   struct remote_state *remote_state_new(void)
>   {
> -	struct remote_state *r = xmalloc(sizeof(*r));
> -
> -	memset(r, 0, sizeof(*r));
> +	struct remote_state *r = xcalloc(1, sizeof(*r));
>   

We have a macro to make this easier and hide the messy details:

	struct remote_state *r;

	CALLOC_ARRAY(r, 1);


Jeff

[PATCH v2] maintenance: use xcalloc instead of xmalloc where possible

[Rose via GitGitGadget]

From: Seija <doremylover123@gmail.com>

We can avoid having to call memset by calling xcalloc directly

Signed-off-by: Seija doremylover123@gmail.com
---
    maintenance: use xcalloc instead of xmalloc where possible
    
    We can avoid having to call memset by calling xcalloc directly
    
    Signed-off-by: Seija doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v2
Pull-Request: https://github.com/git/git/pull/1390

Range-diff vs v1:

 1:  f56282194a7 ! 1:  ee8a7af6435 maintenance: use xcalloc instead of xmalloc where possible
     @@ remote.c: void apply_push_cas(struct push_cas_option *cas,
       struct remote_state *remote_state_new(void)
       {
      -	struct remote_state *r = xmalloc(sizeof(*r));
     --
     ++	struct remote_state *r;
     + 
      -	memset(r, 0, sizeof(*r));
     -+	struct remote_state *r = xcalloc(1, sizeof(*r));
     ++	CALLOC_ARRAY(r, 1);
       
       	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
       	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
      
       ## submodule.c ##
     -@@ submodule.c: static const struct submodule *get_non_gitmodules_submodule(const char *path)
     +@@ submodule.c: struct fetch_task {
     +  */
     + static const struct submodule *get_non_gitmodules_submodule(const char *path)
     + {
     +-	struct submodule *ret = NULL;
     ++	struct submodule *ret;
     + 	const char *name = default_name_or_path(path);
     + 
       	if (!name)
       		return NULL;
       
      -	ret = xmalloc(sizeof(*ret));
      -	memset(ret, 0, sizeof(*ret));
     -+	ret = xcalloc(1, sizeof(*ret));
     ++	CALLOC_ARRAY(ret, 1);
       	ret->path = name;
       	ret->name = name;
       


 remote.c    | 4 ++--
 submodule.c | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index 60869beebe7..475a1d18af0 100644
--- a/remote.c
+++ b/remote.c
@@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
 
 struct remote_state *remote_state_new(void)
 {
-	struct remote_state *r = xmalloc(sizeof(*r));
+	struct remote_state *r;
 
-	memset(r, 0, sizeof(*r));
+	CALLOC_ARRAY(r, 1);
 
 	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
 	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
diff --git a/submodule.c b/submodule.c
index 8ac2fca855d..d43774c7527 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1458,14 +1458,13 @@ struct fetch_task {
  */
 static const struct submodule *get_non_gitmodules_submodule(const char *path)
 {
-	struct submodule *ret = NULL;
+	struct submodule *ret;
 	const char *name = default_name_or_path(path);
 
 	if (!name)
 		return NULL;
 
-	ret = xmalloc(sizeof(*ret));
-	memset(ret, 0, sizeof(*ret));
+	CALLOC_ARRAY(ret, 1);
 	ret->path = name;
 	ret->name = name;
 

base-commit: 805265fcf7a737664a8321aaf4a0587b78435184
-- 
gitgitgadget

[PATCH v3] maintenance: use xcalloc instead of xmalloc where possible

[Rose via GitGitGadget]

From: Seija <doremylover123@gmail.com>

We can avoid having to call memset by calling xcalloc directly

Signed-off-by: Seija doremylover123@gmail.com
---
    maintenance: use xcalloc instead of xmalloc where possible
    
    We can avoid having to call memset by calling xcalloc directly
    
    Signed-off-by: Seija doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v3
Pull-Request: https://github.com/git/git/pull/1390

Range-diff vs v2:

 1:  ee8a7af6435 ! 1:  b5cfec60048 maintenance: use xcalloc instead of xmalloc where possible
     @@ Commit message
      
          Signed-off-by: Seija doremylover123@gmail.com
      
     + ## builtin/pack-redundant.c ##
     +@@ builtin/pack-redundant.c: static inline struct llist_item *llist_item_get(void)
     + 		new_item = free_nodes;
     + 		free_nodes = free_nodes->next;
     + 	} else {
     +-		int i = 1;
     ++		size_t i = 1;
     + 		ALLOC_ARRAY(new_item, BLKSIZE);
     + 		for (; i < BLKSIZE; i++)
     + 			llist_item_put(&new_item[i]);
     +@@ builtin/pack-redundant.c: static inline struct llist_item *llist_item_get(void)
     + 
     + static inline void llist_init(struct llist **list)
     + {
     +-	*list = xmalloc(sizeof(struct llist));
     +-	(*list)->front = (*list)->back = NULL;
     +-	(*list)->size = 0;
     ++	CALLOC_ARRAY(*list, 1);
     + }
     + 
     + static struct llist * llist_copy(struct llist *list)
     +
       ## remote.c ##
      @@ remote.c: void apply_push_cas(struct push_cas_option *cas,
       
     @@ submodule.c: struct fetch_task {
       	ret->path = name;
       	ret->name = name;
       
     +@@ submodule.c: static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
     + 					    const char *path,
     + 					    const struct object_id *treeish_name)
     + {
     +-	struct fetch_task *task = xmalloc(sizeof(*task));
     +-	memset(task, 0, sizeof(*task));
     ++	struct fetch_task *task;
     ++
     ++	CALLOC_ARRAY(task, 1);
     + 
     + 	task->sub = submodule_from_path(spf->r, treeish_name, path);
     + 
     +
     + ## xdiff/xutils.c ##
     +@@ xdiff/xutils.c: void *xdl_cha_alloc(chastore_t *cha) {
     + 	void *data;
     + 
     + 	if (!(ancur = cha->ancur) || ancur->icurr == cha->nsize) {
     +-		if (!(ancur = (chanode_t *) xdl_malloc(sizeof(chanode_t) + cha->nsize))) {
     ++		if (!(ancur = (chanode_t *) xdl_calloc(1, sizeof(chanode_t) + cha->nsize))) {
     + 
     + 			return NULL;
     + 		}
     +-		ancur->icurr = 0;
     +-		ancur->next = NULL;
     + 		if (cha->tail)
     + 			cha->tail->next = ancur;
     + 		if (!cha->head)


 builtin/pack-redundant.c |  6 ++----
 remote.c                 |  4 ++--
 submodule.c              | 10 +++++-----
 xdiff/xutils.c           |  4 +---
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index ecd49ca268f..0e184bb5212 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -51,7 +51,7 @@ static inline struct llist_item *llist_item_get(void)
 		new_item = free_nodes;
 		free_nodes = free_nodes->next;
 	} else {
-		int i = 1;
+		size_t i = 1;
 		ALLOC_ARRAY(new_item, BLKSIZE);
 		for (; i < BLKSIZE; i++)
 			llist_item_put(&new_item[i]);
@@ -61,9 +61,7 @@ static inline struct llist_item *llist_item_get(void)
 
 static inline void llist_init(struct llist **list)
 {
-	*list = xmalloc(sizeof(struct llist));
-	(*list)->front = (*list)->back = NULL;
-	(*list)->size = 0;
+	CALLOC_ARRAY(*list, 1);
 }
 
 static struct llist * llist_copy(struct llist *list)
diff --git a/remote.c b/remote.c
index 60869beebe7..475a1d18af0 100644
--- a/remote.c
+++ b/remote.c
@@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
 
 struct remote_state *remote_state_new(void)
 {
-	struct remote_state *r = xmalloc(sizeof(*r));
+	struct remote_state *r;
 
-	memset(r, 0, sizeof(*r));
+	CALLOC_ARRAY(r, 1);
 
 	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
 	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
diff --git a/submodule.c b/submodule.c
index 8ac2fca855d..015102a83d6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1458,14 +1458,13 @@ struct fetch_task {
  */
 static const struct submodule *get_non_gitmodules_submodule(const char *path)
 {
-	struct submodule *ret = NULL;
+	struct submodule *ret;
 	const char *name = default_name_or_path(path);
 
 	if (!name)
 		return NULL;
 
-	ret = xmalloc(sizeof(*ret));
-	memset(ret, 0, sizeof(*ret));
+	CALLOC_ARRAY(ret, 1);
 	ret->path = name;
 	ret->name = name;
 
@@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
 					    const char *path,
 					    const struct object_id *treeish_name)
 {
-	struct fetch_task *task = xmalloc(sizeof(*task));
-	memset(task, 0, sizeof(*task));
+	struct fetch_task *task;
+
+	CALLOC_ARRAY(task, 1);
 
 	task->sub = submodule_from_path(spf->r, treeish_name, path);
 
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 9e36f24875d..c19bc441a96 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -98,12 +98,10 @@ void *xdl_cha_alloc(chastore_t *cha) {
 	void *data;
 
 	if (!(ancur = cha->ancur) || ancur->icurr == cha->nsize) {
-		if (!(ancur = (chanode_t *) xdl_malloc(sizeof(chanode_t) + cha->nsize))) {
+		if (!(ancur = (chanode_t *) xdl_calloc(1, sizeof(chanode_t) + cha->nsize))) {
 
 			return NULL;
 		}
-		ancur->icurr = 0;
-		ancur->next = NULL;
 		if (cha->tail)
 			cha->tail->next = ancur;
 		if (!cha->head)

base-commit: 805265fcf7a737664a8321aaf4a0587b78435184
-- 
gitgitgadget

[PATCH v4] maintenance: use calloc instead of malloc where possible

[Rose via GitGitGadget]

From: Seija <doremylover123@gmail.com>

We can avoid having to call memset by calling calloc directly

Signed-off-by: Seija doremylover123@gmail.com
---
    maintenance: use calloc instead of malloc where possible
    
    We can avoid having to call memset by calling calloc directly
    
    Signed-off-by: Seija doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v4
Pull-Request: https://github.com/git/git/pull/1390

Range-diff vs v3:

 1:  b5cfec60048 ! 1:  3cd6b1eab13 maintenance: use xcalloc instead of xmalloc where possible
     @@ Metadata
      Author: Seija <doremylover123@gmail.com>
      
       ## Commit message ##
     -    maintenance: use xcalloc instead of xmalloc where possible
     +    maintenance: use calloc instead of malloc where possible
      
     -    We can avoid having to call memset by calling xcalloc directly
     +    We can avoid having to call memset by calling calloc directly
      
          Signed-off-by: Seija doremylover123@gmail.com
      


 builtin/pack-redundant.c |  6 ++----
 remote.c                 |  4 ++--
 submodule.c              | 10 +++++-----
 xdiff/xutils.c           |  4 +---
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index ecd49ca268f..0e184bb5212 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -51,7 +51,7 @@ static inline struct llist_item *llist_item_get(void)
 		new_item = free_nodes;
 		free_nodes = free_nodes->next;
 	} else {
-		int i = 1;
+		size_t i = 1;
 		ALLOC_ARRAY(new_item, BLKSIZE);
 		for (; i < BLKSIZE; i++)
 			llist_item_put(&new_item[i]);
@@ -61,9 +61,7 @@ static inline struct llist_item *llist_item_get(void)
 
 static inline void llist_init(struct llist **list)
 {
-	*list = xmalloc(sizeof(struct llist));
-	(*list)->front = (*list)->back = NULL;
-	(*list)->size = 0;
+	CALLOC_ARRAY(*list, 1);
 }
 
 static struct llist * llist_copy(struct llist *list)
diff --git a/remote.c b/remote.c
index 60869beebe7..475a1d18af0 100644
--- a/remote.c
+++ b/remote.c
@@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
 
 struct remote_state *remote_state_new(void)
 {
-	struct remote_state *r = xmalloc(sizeof(*r));
+	struct remote_state *r;
 
-	memset(r, 0, sizeof(*r));
+	CALLOC_ARRAY(r, 1);
 
 	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
 	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
diff --git a/submodule.c b/submodule.c
index 8ac2fca855d..015102a83d6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1458,14 +1458,13 @@ struct fetch_task {
  */
 static const struct submodule *get_non_gitmodules_submodule(const char *path)
 {
-	struct submodule *ret = NULL;
+	struct submodule *ret;
 	const char *name = default_name_or_path(path);
 
 	if (!name)
 		return NULL;
 
-	ret = xmalloc(sizeof(*ret));
-	memset(ret, 0, sizeof(*ret));
+	CALLOC_ARRAY(ret, 1);
 	ret->path = name;
 	ret->name = name;
 
@@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
 					    const char *path,
 					    const struct object_id *treeish_name)
 {
-	struct fetch_task *task = xmalloc(sizeof(*task));
-	memset(task, 0, sizeof(*task));
+	struct fetch_task *task;
+
+	CALLOC_ARRAY(task, 1);
 
 	task->sub = submodule_from_path(spf->r, treeish_name, path);
 
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 9e36f24875d..c19bc441a96 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -98,12 +98,10 @@ void *xdl_cha_alloc(chastore_t *cha) {
 	void *data;
 
 	if (!(ancur = cha->ancur) || ancur->icurr == cha->nsize) {
-		if (!(ancur = (chanode_t *) xdl_malloc(sizeof(chanode_t) + cha->nsize))) {
+		if (!(ancur = (chanode_t *) xdl_calloc(1, sizeof(chanode_t) + cha->nsize))) {
 
 			return NULL;
 		}
-		ancur->icurr = 0;
-		ancur->next = NULL;
 		if (cha->tail)
 			cha->tail->next = ancur;
 		if (!cha->head)

base-commit: 805265fcf7a737664a8321aaf4a0587b78435184
-- 
gitgitgadget

Re: [PATCH v4] maintenance: use calloc instead of malloc where possible

[Junio C Hamano]

"Rose via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Seija <doremylover123@gmail.com>
>
> We can avoid having to call memset by calling calloc directly

What is not explained here is how that makes the code better in what
way.  Reduced line count?  Reduced cycle count?  Reduced line count?

The reason I ask is because the patch touches codepaths that do not
call memset() after malloc(), which the above may explain.

Also, the patch does not use calloc() directly, but uses
CALLOC_ARRAY() doesn't it?

>      +    maintenance: use calloc instead of malloc where possible

This is not about "git maintenance", is it?  Why is that subsystem
specifically named here?

This is not about the patch, because Signed-off-by: line has a legal
meaning (also see SubmittingPatches[[real-name]]), would you mind
explaining what is going on with your name?  The e-mailed patches
come from "Rose", but the patch author identifies themselves as
"Seija".

> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index ecd49ca268f..0e184bb5212 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c
> @@ -51,7 +51,7 @@ static inline struct llist_item *llist_item_get(void)
>  		new_item = free_nodes;
>  		free_nodes = free_nodes->next;
>  	} else {
> -		int i = 1;
> +		size_t i = 1;
>  		ALLOC_ARRAY(new_item, BLKSIZE);
>  		for (; i < BLKSIZE; i++)
>  			llist_item_put(&new_item[i]);

This is unrelated change, isn't it?

> @@ -61,9 +61,7 @@ static inline struct llist_item *llist_item_get(void)
>  
>  static inline void llist_init(struct llist **list)
>  {
> -	*list = xmalloc(sizeof(struct llist));
> -	(*list)->front = (*list)->back = NULL;
> -	(*list)->size = 0;
> +	CALLOC_ARRAY(*list, 1);
>  }

I am somewhat torn on this one.

The original makes it crystal clear that the initial state of the
llist data structure is that the .front and the .back pointers that
point at the head and the tail of a linearly linked list point at
NULL and the .size member indicates there is zero elements on the
list.  IOW, unlike a mindless "memset()" in a

	x = malloc(sizeof(...));
	memset(x, '\0', sizeof(...));

sequence, the way members are cleared was meaningful in the
original.

Using CALLOC_ARRAY() makes it as bad as use of memset() that blindly
fills the memory reason with NUL bytes.  Surely a NULL pointer may
have the same bit representation as a region of memory filled with
NUL, and a size_t integer whose value is zero may also share the
same bit representation, but it lost clarity of the original.

On the other hand, if we are willing to accept the conciseness, and
accept the "\0 filled memory region is the naturally initialized
state for this structure" convention, then I do not see the value of
having a separate llist_init() helper function.  It is used only in
a handful places in this single file, so getting rid of the helper
and writing CALLOC_ARRAY(x, 1) to clear each instance of the structure
it is used to clear may not be a bad thing.

And the presented solution is neither.  Again, I'd prefer to keep
the original, but if we must use CALLOC_ARRAY() to replace it, then
I'd prefer to see that the helper function to be removed.

> diff --git a/remote.c b/remote.c
> index 60869beebe7..475a1d18af0 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
>  
>  struct remote_state *remote_state_new(void)
>  {
> -	struct remote_state *r = xmalloc(sizeof(*r));
> +	struct remote_state *r;
>  
> -	memset(r, 0, sizeof(*r));
> +	CALLOC_ARRAY(r, 1);

This IS an improvement.  We do the mindless clearing either way, it
is just shorter and easier to follow.

> diff --git a/submodule.c b/submodule.c
> index 8ac2fca855d..015102a83d6 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1458,14 +1458,13 @@ struct fetch_task {
>   */
>  static const struct submodule *get_non_gitmodules_submodule(const char *path)
>  {
> -	struct submodule *ret = NULL;
> +	struct submodule *ret;
>  	const char *name = default_name_or_path(path);
>  
>  	if (!name)
>  		return NULL;
>  
> -	ret = xmalloc(sizeof(*ret));
> -	memset(ret, 0, sizeof(*ret));
> +	CALLOC_ARRAY(ret, 1);

Ditto.

> @@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
>  					    const char *path,
>  					    const struct object_id *treeish_name)
>  {
> -	struct fetch_task *task = xmalloc(sizeof(*task));
> -	memset(task, 0, sizeof(*task));
> +	struct fetch_task *task;
> +
> +	CALLOC_ARRAY(task, 1);
>  
>  	task->sub = submodule_from_path(spf->r, treeish_name, path);

Ditto.

> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index 9e36f24875d..c19bc441a96 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -98,12 +98,10 @@ void *xdl_cha_alloc(chastore_t *cha) {
>  	void *data;
>  
>  	if (!(ancur = cha->ancur) || ancur->icurr == cha->nsize) {
> -		if (!(ancur = (chanode_t *) xdl_malloc(sizeof(chanode_t) + cha->nsize))) {
> +		if (!(ancur = (chanode_t *) xdl_calloc(1, sizeof(chanode_t) + cha->nsize))) {
>  
>  			return NULL;
>  		}
> -		ancur->icurr = 0;
> -		ancur->next = NULL;

I am somewhat negative on this for the same reason why I'd prefer to
keep the llist thing intact.  Also xdiff code is a borrowed code and
I'd rather see us not to touch it unnecessarily.

>  		if (cha->tail)
>  			cha->tail->next = ancur;
>  		if (!cha->head)
>
> base-commit: 805265fcf7a737664a8321aaf4a0587b78435184

Thanks.

[PATCH v5] revision: use calloc instead of malloc where possible

[Rose via GitGitGadget]

From: Seija <doremylover123@gmail.com>

We can avoid having to call memset by calling calloc directly

Signed-off-by: Seija doremylover123@gmail.com
---
    revision: use calloc instead of malloc where possible
    
    We can avoid having to call memset by calling calloc directly
    
    Signed-off-by: Seija doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v5
Pull-Request: https://github.com/git/git/pull/1390

Range-diff vs v4:

 1:  3cd6b1eab13 ! 1:  8072fa30e4f maintenance: use calloc instead of malloc where possible
     @@ Metadata
      Author: Seija <doremylover123@gmail.com>
      
       ## Commit message ##
     -    maintenance: use calloc instead of malloc where possible
     +    revision: use calloc instead of malloc where possible
      
          We can avoid having to call memset by calling calloc directly
      
     @@ Commit message
      
       ## builtin/pack-redundant.c ##
      @@ builtin/pack-redundant.c: static inline struct llist_item *llist_item_get(void)
     - 		new_item = free_nodes;
     - 		free_nodes = free_nodes->next;
     - 	} else {
     --		int i = 1;
     -+		size_t i = 1;
     - 		ALLOC_ARRAY(new_item, BLKSIZE);
     - 		for (; i < BLKSIZE; i++)
     - 			llist_item_put(&new_item[i]);
     -@@ builtin/pack-redundant.c: static inline struct llist_item *llist_item_get(void)
     + 	return new_item;
     + }
       
     - static inline void llist_init(struct llist **list)
     - {
     +-static inline void llist_init(struct llist **list)
     +-{
      -	*list = xmalloc(sizeof(struct llist));
      -	(*list)->front = (*list)->back = NULL;
      -	(*list)->size = 0;
     -+	CALLOC_ARRAY(*list, 1);
     - }
     - 
     +-}
     +-
       static struct llist * llist_copy(struct llist *list)
     + {
     + 	struct llist *ret;
     + 	struct llist_item *new_item, *old_item, *prev;
     + 
     +-	llist_init(&ret);
     ++	CALLOC_ARRAY(ret, 1);
     + 
     + 	if ((ret->size = list->size) == 0)
     + 		return ret;
     +@@ builtin/pack-redundant.c: static void load_all_objects(void)
     + 	struct pack_list *pl = local_packs;
     + 	struct llist_item *hint, *l;
     + 
     +-	llist_init(&all_objects);
     ++	CALLOC_ARRAY(all_objects, 1);
     + 
     + 	while (pl) {
     + 		hint = NULL;
     +@@ builtin/pack-redundant.c: static void cmp_local_packs(void)
     + 
     + 	/* only one packfile */
     + 	if (!pl->next) {
     +-		llist_init(&pl->unique_objects);
     ++		CALLOC_ARRAY(pl->unique_objects, 1);
     + 		return;
     + 	}
     + 
     +@@ builtin/pack-redundant.c: static struct pack_list * add_pack(struct packed_git *p)
     + 		return NULL;
     + 
     + 	l.pack = p;
     +-	llist_init(&l.remaining_objects);
     ++	CALLOC_ARRAY(l.remaining_objects, 1);
     + 
     + 	if (open_pack_index(p))
     + 		return NULL;
     +@@ builtin/pack-redundant.c: int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
     + 		scan_alt_odb_packs();
     + 
     + 	/* ignore objects given on stdin */
     +-	llist_init(&ignore);
     ++	CALLOC_ARRAY(ignore, 1);
     + 	if (!isatty(0)) {
     + 		while (fgets(buf, sizeof(buf), stdin)) {
     + 			oid = xmalloc(sizeof(*oid));
      
       ## remote.c ##
      @@ remote.c: void apply_push_cas(struct push_cas_option *cas,
     @@ submodule.c: static struct fetch_task *fetch_task_create(struct submodule_parall
       
       	task->sub = submodule_from_path(spf->r, treeish_name, path);
       
     -
     - ## xdiff/xutils.c ##
     -@@ xdiff/xutils.c: void *xdl_cha_alloc(chastore_t *cha) {
     - 	void *data;
     - 
     - 	if (!(ancur = cha->ancur) || ancur->icurr == cha->nsize) {
     --		if (!(ancur = (chanode_t *) xdl_malloc(sizeof(chanode_t) + cha->nsize))) {
     -+		if (!(ancur = (chanode_t *) xdl_calloc(1, sizeof(chanode_t) + cha->nsize))) {
     - 
     - 			return NULL;
     - 		}
     --		ancur->icurr = 0;
     --		ancur->next = NULL;
     - 		if (cha->tail)
     - 			cha->tail->next = ancur;
     - 		if (!cha->head)


 builtin/pack-redundant.c | 17 +++++------------
 remote.c                 |  4 ++--
 submodule.c              | 10 +++++-----
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index ecd49ca268f..ce5be807cf0 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -59,19 +59,12 @@ static inline struct llist_item *llist_item_get(void)
 	return new_item;
 }
 
-static inline void llist_init(struct llist **list)
-{
-	*list = xmalloc(sizeof(struct llist));
-	(*list)->front = (*list)->back = NULL;
-	(*list)->size = 0;
-}
-
 static struct llist * llist_copy(struct llist *list)
 {
 	struct llist *ret;
 	struct llist_item *new_item, *old_item, *prev;
 
-	llist_init(&ret);
+	CALLOC_ARRAY(ret, 1);
 
 	if ((ret->size = list->size) == 0)
 		return ret;
@@ -448,7 +441,7 @@ static void load_all_objects(void)
 	struct pack_list *pl = local_packs;
 	struct llist_item *hint, *l;
 
-	llist_init(&all_objects);
+	CALLOC_ARRAY(all_objects, 1);
 
 	while (pl) {
 		hint = NULL;
@@ -475,7 +468,7 @@ static void cmp_local_packs(void)
 
 	/* only one packfile */
 	if (!pl->next) {
-		llist_init(&pl->unique_objects);
+		CALLOC_ARRAY(pl->unique_objects, 1);
 		return;
 	}
 
@@ -512,7 +505,7 @@ static struct pack_list * add_pack(struct packed_git *p)
 		return NULL;
 
 	l.pack = p;
-	llist_init(&l.remaining_objects);
+	CALLOC_ARRAY(l.remaining_objects, 1);
 
 	if (open_pack_index(p))
 		return NULL;
@@ -620,7 +613,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 		scan_alt_odb_packs();
 
 	/* ignore objects given on stdin */
-	llist_init(&ignore);
+	CALLOC_ARRAY(ignore, 1);
 	if (!isatty(0)) {
 		while (fgets(buf, sizeof(buf), stdin)) {
 			oid = xmalloc(sizeof(*oid));
diff --git a/remote.c b/remote.c
index 60869beebe7..475a1d18af0 100644
--- a/remote.c
+++ b/remote.c
@@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
 
 struct remote_state *remote_state_new(void)
 {
-	struct remote_state *r = xmalloc(sizeof(*r));
+	struct remote_state *r;
 
-	memset(r, 0, sizeof(*r));
+	CALLOC_ARRAY(r, 1);
 
 	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
 	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
diff --git a/submodule.c b/submodule.c
index 8ac2fca855d..015102a83d6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1458,14 +1458,13 @@ struct fetch_task {
  */
 static const struct submodule *get_non_gitmodules_submodule(const char *path)
 {
-	struct submodule *ret = NULL;
+	struct submodule *ret;
 	const char *name = default_name_or_path(path);
 
 	if (!name)
 		return NULL;
 
-	ret = xmalloc(sizeof(*ret));
-	memset(ret, 0, sizeof(*ret));
+	CALLOC_ARRAY(ret, 1);
 	ret->path = name;
 	ret->name = name;
 
@@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
 					    const char *path,
 					    const struct object_id *treeish_name)
 {
-	struct fetch_task *task = xmalloc(sizeof(*task));
-	memset(task, 0, sizeof(*task));
+	struct fetch_task *task;
+
+	CALLOC_ARRAY(task, 1);
 
 	task->sub = submodule_from_path(spf->r, treeish_name, path);
 

base-commit: 2e71cbbddd64695d43383c25c7a054ac4ff86882
-- 
gitgitgadget

Re: [PATCH v5] revision: use calloc instead of malloc where possible

[Ævar Arnfjörð Bjarmason]

On Tue, Dec 06 2022, Rose via GitGitGadget wrote:

> From: Seija <doremylover123@gmail.com>
>
> We can avoid having to call memset by calling calloc directly
>
> Signed-off-by: Seija doremylover123@gmail.com
> ---
>     revision: use calloc instead of malloc where possible
>     
>     We can avoid having to call memset by calling calloc directly
>     
>     Signed-off-by: Seija doremylover123@gmail.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v5
> Pull-Request: https://github.com/git/git/pull/1390
>
> Range-diff vs v4:
>
>  1:  3cd6b1eab13 ! 1:  8072fa30e4f maintenance: use calloc instead of malloc where possible
>      @@ Metadata
>       Author: Seija <doremylover123@gmail.com>
>       
>        ## Commit message ##
>      -    maintenance: use calloc instead of malloc where possible
>      +    revision: use calloc instead of malloc where possible
>       
>           We can avoid having to call memset by calling calloc directly
>       
>      @@ Commit message
>       
>        ## builtin/pack-redundant.c ##
>       @@ builtin/pack-redundant.c: static inline struct llist_item *llist_item_get(void)
>      - 		new_item = free_nodes;
>      - 		free_nodes = free_nodes->next;
>      - 	} else {
>      --		int i = 1;
>      -+		size_t i = 1;
>      - 		ALLOC_ARRAY(new_item, BLKSIZE);
>      - 		for (; i < BLKSIZE; i++)
>      - 			llist_item_put(&new_item[i]);
>      -@@ builtin/pack-redundant.c: static inline struct llist_item *llist_item_get(void)
>      + 	return new_item;
>      + }
>        
>      - static inline void llist_init(struct llist **list)
>      - {
>      +-static inline void llist_init(struct llist **list)
>      +-{
>       -	*list = xmalloc(sizeof(struct llist));
>       -	(*list)->front = (*list)->back = NULL;
>       -	(*list)->size = 0;
>      -+	CALLOC_ARRAY(*list, 1);
>      - }
>      - 
>      +-}
>      +-
>        static struct llist * llist_copy(struct llist *list)
>      + {
>      + 	struct llist *ret;
>      + 	struct llist_item *new_item, *old_item, *prev;
>      + 
>      +-	llist_init(&ret);
>      ++	CALLOC_ARRAY(ret, 1);
>      + 
>      + 	if ((ret->size = list->size) == 0)
>      + 		return ret;
>      +@@ builtin/pack-redundant.c: static void load_all_objects(void)
>      + 	struct pack_list *pl = local_packs;
>      + 	struct llist_item *hint, *l;
>      + 
>      +-	llist_init(&all_objects);
>      ++	CALLOC_ARRAY(all_objects, 1);
>      + 
>      + 	while (pl) {
>      + 		hint = NULL;
>      +@@ builtin/pack-redundant.c: static void cmp_local_packs(void)
>      + 
>      + 	/* only one packfile */
>      + 	if (!pl->next) {
>      +-		llist_init(&pl->unique_objects);
>      ++		CALLOC_ARRAY(pl->unique_objects, 1);
>      + 		return;
>      + 	}
>      + 
>      +@@ builtin/pack-redundant.c: static struct pack_list * add_pack(struct packed_git *p)
>      + 		return NULL;
>      + 
>      + 	l.pack = p;
>      +-	llist_init(&l.remaining_objects);
>      ++	CALLOC_ARRAY(l.remaining_objects, 1);
>      + 
>      + 	if (open_pack_index(p))
>      + 		return NULL;
>      +@@ builtin/pack-redundant.c: int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
>      + 		scan_alt_odb_packs();
>      + 
>      + 	/* ignore objects given on stdin */
>      +-	llist_init(&ignore);
>      ++	CALLOC_ARRAY(ignore, 1);
>      + 	if (!isatty(0)) {
>      + 		while (fgets(buf, sizeof(buf), stdin)) {
>      + 			oid = xmalloc(sizeof(*oid));
>       
>        ## remote.c ##
>       @@ remote.c: void apply_push_cas(struct push_cas_option *cas,
>      @@ submodule.c: static struct fetch_task *fetch_task_create(struct submodule_parall
>        
>        	task->sub = submodule_from_path(spf->r, treeish_name, path);
>        
>      -
>      - ## xdiff/xutils.c ##
>      -@@ xdiff/xutils.c: void *xdl_cha_alloc(chastore_t *cha) {
>      - 	void *data;
>      - 
>      - 	if (!(ancur = cha->ancur) || ancur->icurr == cha->nsize) {
>      --		if (!(ancur = (chanode_t *) xdl_malloc(sizeof(chanode_t) + cha->nsize))) {
>      -+		if (!(ancur = (chanode_t *) xdl_calloc(1, sizeof(chanode_t) + cha->nsize))) {
>      - 
>      - 			return NULL;
>      - 		}
>      --		ancur->icurr = 0;
>      --		ancur->next = NULL;
>      - 		if (cha->tail)
>      - 			cha->tail->next = ancur;
>      - 		if (!cha->head)
>
>
>  builtin/pack-redundant.c | 17 +++++------------
>  remote.c                 |  4 ++--
>  submodule.c              | 10 +++++-----
>  3 files changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index ecd49ca268f..ce5be807cf0 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c
> @@ -59,19 +59,12 @@ static inline struct llist_item *llist_item_get(void)
>  	return new_item;
>  }
>  
> -static inline void llist_init(struct llist **list)
> -{
> -	*list = xmalloc(sizeof(struct llist));
> -	(*list)->front = (*list)->back = NULL;
> -	(*list)->size = 0;
> -}
> -
>  static struct llist * llist_copy(struct llist *list)
>  {
>  	struct llist *ret;
>  	struct llist_item *new_item, *old_item, *prev;
>  
> -	llist_init(&ret);
> +	CALLOC_ARRAY(ret, 1);
>  
>  	if ((ret->size = list->size) == 0)
>  		return ret;
> @@ -448,7 +441,7 @@ static void load_all_objects(void)
>  	struct pack_list *pl = local_packs;
>  	struct llist_item *hint, *l;
>  
> -	llist_init(&all_objects);
> +	CALLOC_ARRAY(all_objects, 1);
>  
>  	while (pl) {
>  		hint = NULL;
> @@ -475,7 +468,7 @@ static void cmp_local_packs(void)
>  
>  	/* only one packfile */
>  	if (!pl->next) {
> -		llist_init(&pl->unique_objects);
> +		CALLOC_ARRAY(pl->unique_objects, 1);
>  		return;
>  	}
>  
> @@ -512,7 +505,7 @@ static struct pack_list * add_pack(struct packed_git *p)
>  		return NULL;
>  
>  	l.pack = p;
> -	llist_init(&l.remaining_objects);
> +	CALLOC_ARRAY(l.remaining_objects, 1);
>  
>  	if (open_pack_index(p))
>  		return NULL;
> @@ -620,7 +613,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
>  		scan_alt_odb_packs();
>  
>  	/* ignore objects given on stdin */
> -	llist_init(&ignore);
> +	CALLOC_ARRAY(ignore, 1);
>  	if (!isatty(0)) {
>  		while (fgets(buf, sizeof(buf), stdin)) {
>  			oid = xmalloc(sizeof(*oid));
> diff --git a/remote.c b/remote.c
> index 60869beebe7..475a1d18af0 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
>  
>  struct remote_state *remote_state_new(void)
>  {
> -	struct remote_state *r = xmalloc(sizeof(*r));
> +	struct remote_state *r;
>  
> -	memset(r, 0, sizeof(*r));
> +	CALLOC_ARRAY(r, 1);
>  
>  	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
>  	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
> diff --git a/submodule.c b/submodule.c
> index 8ac2fca855d..015102a83d6 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1458,14 +1458,13 @@ struct fetch_task {
>   */
>  static const struct submodule *get_non_gitmodules_submodule(const char *path)
>  {
> -	struct submodule *ret = NULL;
> +	struct submodule *ret;
>  	const char *name = default_name_or_path(path);
>  
>  	if (!name)
>  		return NULL;
>  
> -	ret = xmalloc(sizeof(*ret));
> -	memset(ret, 0, sizeof(*ret));
> +	CALLOC_ARRAY(ret, 1);
>  	ret->path = name;
>  	ret->name = name;
>  
> @@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
>  					    const char *path,
>  					    const struct object_id *treeish_name)
>  {
> -	struct fetch_task *task = xmalloc(sizeof(*task));
> -	memset(task, 0, sizeof(*task));
> +	struct fetch_task *task;
> +
> +	CALLOC_ARRAY(task, 1);
>  
>  	task->sub = submodule_from_path(spf->r, treeish_name, path);
>  
>
> base-commit: 2e71cbbddd64695d43383c25c7a054ac4ff86882

This is partially some of the sentiments Junio did in the v4 review
(although I looked at this before reading that).

I think calloc-ing or memset-ing a struct is going in the opposite
direction of where we've been trending, which is to have explicit
initializers. Sometimes it's justified, but in the "pack-redundant.c"
case I think it's probably better & more future-proof to keep it, and
get rid of the odd patter nof passing in a ** to an init function to
have it alloc for us.

I.e. this (there's some stray new free() in there from testing, sorry,
but those are also bugs we should fix...):

	diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
	index ecd49ca268f..af91d643dd2 100644
	--- a/builtin/pack-redundant.c
	+++ b/builtin/pack-redundant.c
	@@ -27,6 +27,7 @@ static struct llist {
	 	struct llist_item *back;
	 	size_t size;
	 } *all_objects; /* all objects which must be present in local packfiles */
	+#define LLIST_INIT { 0 }
	 
	 static struct pack_list {
	 	struct pack_list *next;
	@@ -59,19 +60,18 @@ static inline struct llist_item *llist_item_get(void)
	 	return new_item;
	 }
	 
	-static inline void llist_init(struct llist **list)
	+static inline void llist_init(struct llist *list)
	 {
	-	*list = xmalloc(sizeof(struct llist));
	-	(*list)->front = (*list)->back = NULL;
	-	(*list)->size = 0;
	+	struct llist blank = LLIST_INIT;
	+	memcpy(list, &blank, sizeof(*list));
	 }
	 
	 static struct llist * llist_copy(struct llist *list)
	 {
	-	struct llist *ret;
	+	struct llist *ret = xmalloc(sizeof(struct llist));
	 	struct llist_item *new_item, *old_item, *prev;
	 
	-	llist_init(&ret);
	+	llist_init(ret);
	 
	 	if ((ret->size = list->size) == 0)
	 		return ret;
	@@ -420,6 +420,7 @@ static void minimize(struct pack_list **min)
	 
	 	unique_pack_objects = llist_copy(all_objects);
	 	llist_sorted_difference_inplace(unique_pack_objects, missing);
	+	free(missing);
	 
	 	/* remove unique pack objects from the non_unique packs */
	 	pl = non_unique;
	@@ -447,8 +448,9 @@ static void load_all_objects(void)
	 {
	 	struct pack_list *pl = local_packs;
	 	struct llist_item *hint, *l;
	+	all_objects = xmalloc(sizeof(struct llist));
	 
	-	llist_init(&all_objects);
	+	llist_init(all_objects);
	 
	 	while (pl) {
	 		hint = NULL;
	@@ -475,7 +477,8 @@ static void cmp_local_packs(void)
	 
	 	/* only one packfile */
	 	if (!pl->next) {
	-		llist_init(&pl->unique_objects);
	+		pl->unique_objects = xmalloc(sizeof(struct llist));
	+		llist_init(pl->unique_objects);
	 		return;
	 	}
	 
	@@ -512,7 +515,8 @@ static struct pack_list * add_pack(struct packed_git *p)
	 		return NULL;
	 
	 	l.pack = p;
	-	llist_init(&l.remaining_objects);
	+	l.remaining_objects = xmalloc(sizeof(struct llist));
	+	llist_init(l.remaining_objects);
	 
	 	if (open_pack_index(p))
	 		return NULL;
	@@ -562,7 +566,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
	 	int i;
	 	int i_still_use_this = 0;
	 	struct pack_list *min = NULL, *red, *pl;
	-	struct llist *ignore;
	+	struct llist *ignore = xmalloc(sizeof(struct llist));
	 	struct object_id *oid;
	 	char buf[GIT_MAX_HEXSZ + 2]; /* hex hash + \n + \0 */
	 
	@@ -620,7 +624,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
	 		scan_alt_odb_packs();
	 
	 	/* ignore objects given on stdin */
	-	llist_init(&ignore);
	+	llist_init(ignore);
	 	if (!isatty(0)) {
	 		while (fgets(buf, sizeof(buf), stdin)) {
	 			oid = xmalloc(sizeof(*oid));
	@@ -635,6 +639,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
	 		llist_sorted_difference_inplace(pl->remaining_objects, ignore);
	 		pl = pl->next;
	 	}
	+	free(ignore);
	 
	 	cmp_local_packs();

Now, for:

> @@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
>  					    const char *path,
>  					    const struct object_id *treeish_name)
>  {
> -	struct fetch_task *task = xmalloc(sizeof(*task));
> -	memset(task, 0, sizeof(*task));
> +	struct fetch_task *task;
> +
> +	CALLOC_ARRAY(task, 1);
>  
>  	task->sub = submodule_from_path(spf->r, treeish_name, path);

This is no worse than before, but I think we're just converting here
from one bad pattern to another.

The "struct fetch_task" contains a "struct strvec", for many of our
structs we play it fast and loose with whether you can calloc() it, but
for "struct strbuf", "struct strvec" etc. we have an "empty_strvec" (or
"slopbuf" etc.), so we *really* want those to be properly init'd.

AFAICT we're just lucky that it happens to work with the strvec API in
this case, but if this was a strbuf it could easily segfault etc.

So the better fix here since we're spending review time on it isn't to
move from one memset() equivalent pattern to another, but actually to
start properly initializing this.

Finally, this commit just seems to be all over the place in what it's
changing. We have a bunch of:

	x = xmalloc(...);
	memset(x, 0, ...);

In our tree, it's not clear why these are being picked out in
particular, or what they have to do with each other.

I think that if we're proposing to refactor these doing so with
coccinelle is a much better thing to do. There's a parallel thread about
that over at:
https://lore.kernel.org/git/6694c52b38674859eb0390c7f62da1209a8d8ec3.1670266373.git.me@ttaylorr.com/

[PATCH v6] revision: use calloc instead of malloc where possible

[Rose via GitGitGadget]

From: Seija <doremylover123@gmail.com>

We can avoid having to call memset by calling calloc

Signed-off-by: Seija doremylover123@gmail.com
---
    revision: use calloc instead of malloc where possible
    
    We can avoid having to call memset by calling calloc
    
    Signed-off-by: Seija doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1390%2FAtariDreams%2Fcalloc-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1390/AtariDreams/calloc-v6
Pull-Request: https://github.com/git/git/pull/1390

Range-diff vs v5:

 1:  8072fa30e4f ! 1:  e012cf5c158 revision: use calloc instead of malloc where possible
     @@ Metadata
       ## Commit message ##
          revision: use calloc instead of malloc where possible
      
     -    We can avoid having to call memset by calling calloc directly
     +    We can avoid having to call memset by calling calloc
      
          Signed-off-by: Seija doremylover123@gmail.com
      


 builtin/pack-redundant.c | 17 +++++------------
 remote.c                 |  4 ++--
 submodule.c              | 10 +++++-----
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index ecd49ca268f..ce5be807cf0 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -59,19 +59,12 @@ static inline struct llist_item *llist_item_get(void)
 	return new_item;
 }
 
-static inline void llist_init(struct llist **list)
-{
-	*list = xmalloc(sizeof(struct llist));
-	(*list)->front = (*list)->back = NULL;
-	(*list)->size = 0;
-}
-
 static struct llist * llist_copy(struct llist *list)
 {
 	struct llist *ret;
 	struct llist_item *new_item, *old_item, *prev;
 
-	llist_init(&ret);
+	CALLOC_ARRAY(ret, 1);
 
 	if ((ret->size = list->size) == 0)
 		return ret;
@@ -448,7 +441,7 @@ static void load_all_objects(void)
 	struct pack_list *pl = local_packs;
 	struct llist_item *hint, *l;
 
-	llist_init(&all_objects);
+	CALLOC_ARRAY(all_objects, 1);
 
 	while (pl) {
 		hint = NULL;
@@ -475,7 +468,7 @@ static void cmp_local_packs(void)
 
 	/* only one packfile */
 	if (!pl->next) {
-		llist_init(&pl->unique_objects);
+		CALLOC_ARRAY(pl->unique_objects, 1);
 		return;
 	}
 
@@ -512,7 +505,7 @@ static struct pack_list * add_pack(struct packed_git *p)
 		return NULL;
 
 	l.pack = p;
-	llist_init(&l.remaining_objects);
+	CALLOC_ARRAY(l.remaining_objects, 1);
 
 	if (open_pack_index(p))
 		return NULL;
@@ -620,7 +613,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 		scan_alt_odb_packs();
 
 	/* ignore objects given on stdin */
-	llist_init(&ignore);
+	CALLOC_ARRAY(ignore, 1);
 	if (!isatty(0)) {
 		while (fgets(buf, sizeof(buf), stdin)) {
 			oid = xmalloc(sizeof(*oid));
diff --git a/remote.c b/remote.c
index 60869beebe7..475a1d18af0 100644
--- a/remote.c
+++ b/remote.c
@@ -2741,9 +2741,9 @@ void apply_push_cas(struct push_cas_option *cas,
 
 struct remote_state *remote_state_new(void)
 {
-	struct remote_state *r = xmalloc(sizeof(*r));
+	struct remote_state *r;
 
-	memset(r, 0, sizeof(*r));
+	CALLOC_ARRAY(r, 1);
 
 	hashmap_init(&r->remotes_hash, remotes_hash_cmp, NULL, 0);
 	hashmap_init(&r->branches_hash, branches_hash_cmp, NULL, 0);
diff --git a/submodule.c b/submodule.c
index 8ac2fca855d..015102a83d6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1458,14 +1458,13 @@ struct fetch_task {
  */
 static const struct submodule *get_non_gitmodules_submodule(const char *path)
 {
-	struct submodule *ret = NULL;
+	struct submodule *ret;
 	const char *name = default_name_or_path(path);
 
 	if (!name)
 		return NULL;
 
-	ret = xmalloc(sizeof(*ret));
-	memset(ret, 0, sizeof(*ret));
+	CALLOC_ARRAY(ret, 1);
 	ret->path = name;
 	ret->name = name;
 
@@ -1504,8 +1503,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
 					    const char *path,
 					    const struct object_id *treeish_name)
 {
-	struct fetch_task *task = xmalloc(sizeof(*task));
-	memset(task, 0, sizeof(*task));
+	struct fetch_task *task;
+
+	CALLOC_ARRAY(task, 1);
 
 	task->sub = submodule_from_path(spf->r, treeish_name, path);
 

base-commit: 2e71cbbddd64695d43383c25c7a054ac4ff86882
-- 
gitgitgadget

Re: [PATCH v6] revision: use calloc instead of malloc where possible

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Tue, Dec 06, 2022 at 07:53:14PM +0000, Rose via GitGitGadget wrote:
> From: Seija <doremylover123@gmail.com>
> 
> We can avoid having to call memset by calling calloc

Who are "we"?

Please avoid using first-person pronouns (I and we), since these are
ambiguous in context of many entities (individuals/companies)
participating in development. Instead, write in imperative mood and
passive voice, e.g. "Avoid the need to call memset by allocating with
calloc() instead of malloc().".

Even then, what are justifications of malloc() -> calloc() conversion
other than the described above?

> 
> Signed-off-by: Seija doremylover123@gmail.com

The SoB doesn't look OK. It should have been
"Signed-off-by: Your Real Name <yourname@what.domain>".

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]