From: Andreas Ericsson <ae@op5.se>
Date: Thu, 26 Jun 2008 06:40:17 GMT
Subject: Re: [PATCH] factorize pack structure allocation
Message-ID: <486339D1.7040706@op5.se>
In-Reply-To: <alpine.LFD.1.10.0806241851420.2979@xanadu.home>

Nicolas Pitre wrote:
> New pack structures are currently allocated in 2 different places
> and all members have to be initialized explicitly.  This is prone
> to errors leading to segmentation faults as found by Teemu Likonen.
> 
> Let's have a common place where this structure is allocated, and have 
> all members implicitly initialized to zero.
> 
> Signed-off-by: Nicolas Pitre <nico@cam.org>
> ---
> diff --git a/sha1_file.c b/sha1_file.c
> index a92f023..c56f674 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -792,18 +792,28 @@ unsigned char* use_pack(struct packed_git *p,
>  	return win->base + offset;
>  }
>  
> +static struct packed_git *alloc_packed_git(int extra)
> +{
> +	struct packed_git *p = xmalloc(sizeof(*p) + extra);
> +	memset(p, 0, sizeof(*p));
> +	p->pack_fd = -1;
> +	return p;
> +}
> +

Minor nit; Use xcalloc() instead. It initializes the allocated area
to zero by default, either by the glibc allocator when it re-uses old
memory, or by the kernel when it's handed to userspace. It's a
micro-optimization, but a worthwhile one imo, especially for repos
with lots and lots of packs (git gc --auto runs galore).

The "calloc() returns nulified memory" dogma conforms to C89 and is
thus about as portable as it gets.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231