{"thread":{"id":"15828","subject":"[PATCH maint] builtin-merge.c: fix memory under-allocation","startedAt":"2008-10-09T00:07:54Z","lastAt":"2008-10-09T15:13:00Z","messageCount":7,"participants":["Brandon Casey","Miklos Vajna","Junio C Hamano","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"92632","messageId":"ZuhLzndR5Uvj7-_NyasP-FHF3AqTyLx-0sjNHMd0BC3LXC8LdLqBeQ@cipher.nrlssc.navy.mil","threadId":"15828","inReplyTo":null,"subject":"[PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-10-09T00:07:54Z","receivedAt":"2008-10-09T00:07:54Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"While we're at it, change the allocation to reference the variable it is\nallocating memory for to try to prevent a similar mistake if the type is\nchanged in the future.\n\nSigned-off-by: Brandon Casey \n---\n\n\nThis causes segfault on IRIX64. I guess the irix compiler doesn't\nover-allocate memory as aggressively as gcc.\n\n-brandon\n\n\n builtin-merge.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex dcaf368..d0bf1fc 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -651,12 +651,12 @@ static void add_strategies(const char *string, unsigned attr)\n static int merge_trivial(void)\n {\n \tunsigned char result_tree[20], result_commit[20];\n-\tstruct commit_list *parent = xmalloc(sizeof(struct commit_list *));\n+\tstruct commit_list *parent = xmalloc(sizeof(*parent));\n \n \twrite_tree_trivial(result_tree);\n \tprintf(\"Wonderful.\\n\");\n \tparent->item = lookup_commit(head);\n-\tparent->next = xmalloc(sizeof(struct commit_list *));\n+\tparent->next = xmalloc(sizeof(*parent->next));\n \tparent->next->item = remoteheads->item;\n \tparent->next->next = NULL;\n \tcommit_tree(merge_msg.buf, result_tree, parent, result_commit);\n-- \n1.6.0.2.446.ge243d\n"},{"id":"92634","messageId":"20081009001727.GP536@genesis.frugalware.org","threadId":"15828","inReplyTo":"ZuhLzndR5Uvj7-_NyasP-FHF3AqTyLx-0sjNHMd0BC3LXC8LdLqBeQ@cipher.nrlssc.navy.mil","subject":"Re: [PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-10-09T00:17:27Z","receivedAt":"2008-10-09T00:17:27Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Oct 08, 2008 at 07:07:54PM -0500, Brandon Casey wrote:\n> While we're at it, change the allocation to reference the variable it is\n> allocating memory for to try to prevent a similar mistake if the type is\n> changed in the future.\n\nIf this is really a problem, then I think it would be good to mention\nthis in Documentation/CodingGuidelines.\n"},{"id":"92636","messageId":"0LPLRUZaEHuEZTri_v38ySJHqYAhrfOpOkyUviUH9eOrx8IXBEAzYA@cipher.nrlssc.navy.mil","threadId":"15828","inReplyTo":"20081009001727.GP536@genesis.frugalware.org","subject":"Re: [PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-10-09T00:27:40Z","receivedAt":"2008-10-09T00:27:40Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Miklos Vajna wrote:\n> On Wed, Oct 08, 2008 at 07:07:54PM -0500, Brandon Casey wrote:\n>> While we're at it, change the allocation to reference the variable it is\n>> allocating memory for to try to prevent a similar mistake if the type is\n>> changed in the future.\n> \n> If this is really a problem, then I think it would be good to mention\n> this in Documentation/CodingGuidelines.\n\nThat's fine. Though I didn't mean to imply that the memory under-allocation\nwas caused by a change in variable type in this case. Re-reading my commit\nmessage, maybe it sounds like that.\n\n Something like this sometimes happens:\n\n- struct a_struct *foo = xmalloc(sizeof(struct a_struct));\n+ struct a_bigr_struct *foo = xmalloc(sizeof(struct a_struct));\n\n which would be avoided if we had started with:\n\n struct a_struct *foo = xmalloc(sizeof(*foo));\n\n-brandon\n"},{"id":"92661","messageId":"7vmyheowcr.fsf@gitster.siamese.dyndns.org","threadId":"15828","inReplyTo":"0LPLRUZaEHuEZTri_v38ySJHqYAhrfOpOkyUviUH9eOrx8IXBEAzYA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-09T11:12:52Z","receivedAt":"2008-10-09T11:12:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Miklos Vajna wrote:\n>> On Wed, Oct 08, 2008 at 07:07:54PM -0500, Brandon Casey wrote:\n>>> While we're at it, change the allocation to reference the variable it is\n>>> allocating memory for to try to prevent a similar mistake if the type is\n>>> changed in the future.\n>> \n>> If this is really a problem, then I think it would be good to mention\n>> this in Documentation/CodingGuidelines.\n>\n> That's fine. Though I didn't mean to imply that the memory under-allocation\n> was caused by a change in variable type in this case. Re-reading my commit\n> message, maybe it sounds like that.\n\nYeah, it does. I was scratching my head and had to read the patch three\ntimes until I got it (yes, I am especially slower than usual today, as the\nreason I am reading mails right now is because I am jetlagged and cannot\nsleep).\n"},{"id":"92671","messageId":"rKU9r5196P_JTKZZkb10yo7uwH_n_7KAuyX6RMOa9jc@cipher.nrlssc.navy.mil","threadId":"15828","inReplyTo":"7vmyheowcr.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-10-09T14:15:02Z","receivedAt":"2008-10-09T14:15:02Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Junio C Hamano wrote:\n> Brandon Casey writes:\n>> I didn't mean to imply that the memory under-allocation\n>> was caused by a change in variable type in this case. Re-reading my commit\n>> message, maybe it sounds like that.\n> \n> Yeah, it does. I was scratching my head and had to read the patch three\n> times until I got it (yes, I am especially slower than usual today, as the\n> reason I am reading mails right now is because I am jetlagged and cannot\n> sleep).\n\nIf it's not too late, maybe this would make a better commit message:\n\n--->8---\nbuiltin-merge.c: allocate correct amount of memory\n\nFix two memory allocation errors which allocate space for a pointer rather\nthan enough space for the structure itself.\n\nThis:\n\n struct commit_list *parent = xmalloc(sizeof(struct commit_list *));\n\nshould have been this:\n\n struct commit_list *parent = xmalloc(sizeof(struct commit_list));\n \nBut while we're at it, change the allocation to reference the variable it is\nallocating memory for to try to prevent a similar mistake, for example if the\ntype is changed, in the future.\n\nSigned-off-by: Brandon Casey \n"},{"id":"92675","messageId":"20081009144957.GT536@genesis.frugalware.org","threadId":"15828","inReplyTo":"rKU9r5196P_JTKZZkb10yo7uwH_n_7KAuyX6RMOa9jc@cipher.nrlssc.navy.mil","subject":"Re: [PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-10-09T14:49:57Z","receivedAt":"2008-10-09T14:49:57Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, Oct 09, 2008 at 09:15:02AM -0500, Brandon Casey wrote:\n> Fix two memory allocation errors which allocate space for a pointer rather\n> than enough space for the structure itself.\n\nAah, I see it now. I thought first that it was just a variable->type\nchange.\n\nAcked-by: Miklos Vajna \n"},{"id":"92677","messageId":"20081009151300.GS8203@spearce.org","threadId":"15828","inReplyTo":"rKU9r5196P_JTKZZkb10yo7uwH_n_7KAuyX6RMOa9jc@cipher.nrlssc.navy.mil","subject":"Re: [PATCH maint] builtin-merge.c: fix memory under-allocation","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-10-09T15:13:00Z","receivedAt":"2008-10-09T15:13:00Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Brandon Casey wrote:\n> Junio C Hamano wrote:\n> > Brandon Casey writes:\n> >> I didn't mean to imply that the memory under-allocation\n> >> was caused by a change in variable type in this case. Re-reading my commit\n> >> message, maybe it sounds like that.\n> > \n> > Yeah, it does. I was scratching my head and had to read the patch three\n> > times until I got it (yes, I am especially slower than usual today, as the\n> > reason I am reading mails right now is because I am jetlagged and cannot\n> > sleep).\n> \n> If it's not too late, maybe this would make a better commit message:\n\nNope, I didn't get around to the patch until now. Better message\nis being used... ;-)\n\nThanks everyone.\n \n> --->8---\n> builtin-merge.c: allocate correct amount of memory\n> \n> Fix two memory allocation errors which allocate space for a pointer rather\n> than enough space for the structure itself.\n> \n> This:\n> \n> struct commit_list *parent = xmalloc(sizeof(struct commit_list *));\n> \n> should have been this:\n> \n> struct commit_list *parent = xmalloc(sizeof(struct commit_list));\n> \n> But while we're at it, change the allocation to reference the variable it is\n> allocating memory for to try to prevent a similar mistake, for example if the\n> type is changed, in the future.\n> \n> Signed-off-by: Brandon Casey \n\n-- \nShawn.\n"}]}