Re: [PATCH maint] builtin-merge.c: fix memory under-allocation

Brandon Casey [off-list ref] writes:

Miklos Vajna wrote:

quoted

On Wed, Oct 08, 2008 at 07:07:54PM -0500, Brandon Casey [off-list ref] wrote:

quoted

While we're at it, change the allocation to reference the variable it is
allocating memory for to try to prevent a similar mistake if the type is
changed in the future.

If this is really a problem, then I think it would be good to mention
this in Documentation/CodingGuidelines.

That's fine. Though I didn't mean to imply that the memory under-allocation
was caused by a change in variable type in this case. Re-reading my commit
message, maybe it sounds like that.

Yeah, it does.  I was scratching my head and had to read the patch three
times until I got it (yes, I am especially slower than usual today, as the
reason I am reading mails right now is because I am jetlagged and cannot
sleep).

Junio C Hamano wrote:

Brandon Casey [off-list ref] writes:

quoted

I didn't mean to imply that the memory under-allocation
was caused by a change in variable type in this case. Re-reading my commit
message, maybe it sounds like that.

Yeah, it does.  I was scratching my head and had to read the patch three
times until I got it (yes, I am especially slower than usual today, as the
reason I am reading mails right now is because I am jetlagged and cannot
sleep).

If it's not too late, maybe this would make a better commit message:

--->8---
builtin-merge.c: allocate correct amount of memory

Fix two memory allocation errors which allocate space for a pointer rather
than enough space for the structure itself.

This:

    struct commit_list *parent = xmalloc(sizeof(struct commit_list *));

should have been this:

    struct commit_list *parent = xmalloc(sizeof(struct commit_list));
    
But while we're at it, change the allocation to reference the variable it is
allocating memory for to try to prevent a similar mistake, for example if the
type is changed, in the future.

Signed-off-by: Brandon Casey <redacted>

On Thu, Oct 09, 2008 at 09:15:02AM -0500, Brandon Casey [off-list ref] wrote:

Fix two memory allocation errors which allocate space for a pointer rather
than enough space for the structure itself.

Aah, I see it now. I thought first that it was just a variable->type
change.

Acked-by: Miklos Vajna <redacted>

Brandon Casey [off-list ref] wrote:

Junio C Hamano wrote:

quoted

Brandon Casey [off-list ref] writes:

quoted

I didn't mean to imply that the memory under-allocation
was caused by a change in variable type in this case. Re-reading my commit
message, maybe it sounds like that.

Yeah, it does.  I was scratching my head and had to read the patch three
times until I got it (yes, I am especially slower than usual today, as the
reason I am reading mails right now is because I am jetlagged and cannot
sleep).

If it's not too late, maybe this would make a better commit message:

Nope, I didn't get around to the patch until now.  Better message
is being used...  ;-)

Thanks everyone.
 

--->8---
builtin-merge.c: allocate correct amount of memory

Fix two memory allocation errors which allocate space for a pointer rather
than enough space for the structure itself.

This:

    struct commit_list *parent = xmalloc(sizeof(struct commit_list *));

should have been this:

    struct commit_list *parent = xmalloc(sizeof(struct commit_list));
    
But while we're at it, change the allocation to reference the variable it is
allocating memory for to try to prevent a similar mistake, for example if the
type is changed, in the future.

Signed-off-by: Brandon Casey <redacted>

-- 
Shawn.