threads / patch / 43468

patch, 5 partsRe: [PATCH 3/5] get_shallow_commits: Avoid memory leak if a commit has been reached already.

Subject: Re: [PATCH 3/5] get_shallow_commits: Avoid memory leak if a commit has been reached already.

## tl;dr

5 messages between Nov 24, 2006 and Nov 25, 2006. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Alexandre Julliard· Nov 24, 2006, 14:58 UTC · lore

[PATCH 3/5] get_shallow_commits: Avoid memory leak if a commit has been reached already.

Signed-off-by: Alexandre Julliard <julliard@winehq.org>
---
 shallow.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Show changes to shallow.c +3 −1

diff --git a/shallow.c b/shallow.c
index 2db1dc4..3d53d17 100644
--- a/shallow.c
+++ b/shallow.c
@@ -60,7 +60,9 @@ struct commit_list *get_shallow_commits(
 					commit = NULL;
 					continue;
 				}
-				commit->util = xcalloc(1, sizeof(int));
+				if (!commit->util)
+					commit->util = xmalloc(sizeof(int));
+				*(int *)commit->util = 0;
 				cur_depth = 0;
 			} else {
 				commit = (struct commit *)

-- 
1.4.4.1.ga335e

-- 
Alexandre Julliard

Anand Kumria· Nov 25, 2006, 17:22 UTC · re: Alexandre Julliard · lore

On Fri, 24 Nov 2006 15:58:50 +0100, Alexandre Julliard wrote:

Show 18 quoted lines

> Signed-off-by: Alexandre Julliard <julliard@winehq.org>
> ---
>  shallow.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/shallow.c b/shallow.c
> index 2db1dc4..3d53d17 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -60,7 +60,9 @@ struct commit_list *get_shallow_commits(
>  					commit = NULL;
>  					continue;
>  				}
> -				commit->util = xcalloc(1, sizeof(int));
> +				if (!commit->util)
> +					commit->util = xmalloc(sizeof(int));
> +				*(int *)commit->util = 0;
>  				cur_depth = 0;

Hi,

Any reason you didn't do:

if (!commit->util)
	commit->util = xcalloc(1, sizeof(int));

That would seem to be the same.

Regards, Anand

Johannes Schindelin· Nov 25, 2006, 18:49 UTC · re: Anand Kumria · lore

Hi,

On Sat, 25 Nov 2006, Anand Kumria wrote:

> Any reason you didn't do:
> 
> if (!commit->util)
> 	commit->util = xcalloc(1, sizeof(int));

xmalloc() does not initialize the memory. Therefore, it is usually preferred if you initialize the memory yourself. In this case, the memory is initialized to 0, even if it is not allocated.

It may be a minor performance issue, but it is a good habit to use xcalloc only if it is needed.

Ciao, Dscho

Junio C Hamano· Nov 25, 2006, 19:44 UTC · re: Johannes Schindelin · lore

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 13 quoted lines

> On Sat, 25 Nov 2006, Anand Kumria wrote:
>
>> Any reason you didn't do:
>> 
>> if (!commit->util)
>> 	commit->util = xcalloc(1, sizeof(int));
>
> xmalloc() does not initialize the memory. Therefore, it is usually 
> preferred if you initialize the memory yourself. In this case, the memory 
> is initialized to 0, even if it is not allocated.
>
> It may be a minor performance issue, but it is a good habit to use xcalloc 
> only if it is needed.

I think you could pretend the commit->util field to be of some kind of int and avoid the allocation altogether ;-).

Johannes Schindelin· Nov 25, 2006, 23:36 UTC · re: Junio C Hamano · lore

Hi,

On Sat, 25 Nov 2006, Junio C Hamano wrote:

> I think you could pretend the commit->util field to be of some kind of 
> int and avoid the allocation altogether ;-).

I actually thought about that, and found it too dangerous. But of course, since we need at least a 32-bit architecture, chances are very high that a pointer is castable to an int.

Ciao, Dscho