{"thread":{"id":"16856","subject":"[PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code","startedAt":"2008-12-24T16:34:36Z","lastAt":"2008-12-30T21:01:44Z","messageCount":4,"participants":["Adeodato Simó","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"98650","messageId":"1230136476-11081-1-git-send-email-dato@net.com.org.es","threadId":"16856","inReplyTo":null,"subject":"[PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2008-12-24T16:34:36Z","receivedAt":"2008-12-24T16:34:36Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"Also, when clearing the \"onelines\" string lists, do not free the \"util\"\nmember: with string_list_append() is not initialized to any value (and\nwas being initialized to NULL previously anyway).\n\nNB: The duplicated code in builtin-shortlog.c predated the appearance of\nstring_list_append().\n\nSigned-off-by: Adeodato Simó <dato@net.com.org.es>\n---\n builtin-shortlog.c |   14 ++------------\n 1 files changed, 2 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex d03f14f..4c5d761 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -36,7 +36,6 @@ static void insert_one_record(struct shortlog *log,\n \tconst char *dot3 = log->common_repo_prefix;\n \tchar *buffer, *p;\n \tstruct string_list_item *item;\n-\tstruct string_list *onelines;\n \tchar namebuf[1024];\n \tsize_t len;\n \tconst char *eol;\n@@ -104,16 +103,7 @@ static void insert_one_record(struct shortlog *log,\n \t\t}\n \t}\n \n-\tonelines = item->util;\n-\tif (onelines->nr >= onelines->alloc) {\n-\t\tonelines->alloc = alloc_nr(onelines->nr);\n-\t\tonelines->items = xrealloc(onelines->items,\n-\t\t\t\tonelines->alloc\n-\t\t\t\t* sizeof(struct string_list_item));\n-\t}\n-\n-\tonelines->items[onelines->nr].util = NULL;\n-\tonelines->items[onelines->nr++].string = buffer;\n+\tstring_list_append(buffer, item->util);\n }\n \n static void read_from_stdin(struct shortlog *log)\n@@ -323,7 +313,7 @@ void shortlog_output(struct shortlog *log)\n \t\t}\n \n \t\tonelines->strdup_strings = 1;\n-\t\tstring_list_clear(onelines, 1);\n+\t\tstring_list_clear(onelines, 0);\n \t\tfree(onelines);\n \t\tlog->list.items[i].util = NULL;\n \t}\n-- \n1.6.0.4\n"},{"id":"98964","messageId":"alpine.DEB.1.00.0812301319140.30769@pacific.mpi-cbg.de","threadId":"16856","inReplyTo":"1230136476-11081-1-git-send-email-dato@net.com.org.es","subject":"Re: [PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-12-30T12:20:18Z","receivedAt":"2008-12-30T12:20:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 24 Dec 2008, Adeodato Simó wrote:\n\n> Also, when clearing the \"onelines\" string lists, do not free the \"util\"\n> member: with string_list_append() is not initialized to any value (and\n> was being initialized to NULL previously anyway).\n> \n> NB: The duplicated code in builtin-shortlog.c predated the appearance of\n> string_list_append().\n> \n> Signed-off-by: Adeodato Simó <dato@net.com.org.es>\n\nFWIW I like the patch, but would like it even more if the strdup() removal \nwas squashed in (with an explanation in the commit message).\n\nCiao,\nDscho"},{"id":"98989","messageId":"1230668722-26394-1-git-send-email-dato@net.com.org.es","threadId":"16856","inReplyTo":"alpine.DEB.1.00.0812301319140.30769@pacific.mpi-cbg.de","subject":"[PATCH v2] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2008-12-30T20:25:22Z","receivedAt":"2008-12-30T20:25:22Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"Make cleanup in insert_one_record() use string_list_append(), instead of\nduplicating its code. Because of this, do not free the \"util\" member when\nclearing the \"onelines\" string lists: with the new code path it is not\ninitialized to any value (was being initialized to NULL previously).\n\nAlso, avoid unnecessary strdup() calls when inserting names in log->list.\nThis list always has \"strdup_strings\" activated, hence strdup'ing namebuf is\nunnecessary. This change also removes a latent memory leak in the old code.\n\nNB: The duplicated code mentioned above predated the appearance of\nstring_list_append().\n\nSigned-off-by: Adeodato Simó <dato@net.com.org.es>\n---\n\n> FWIW I like the patch, but would like it even more if the strdup() removal \n> was squashed in (with an explanation in the commit message).\n\nOk, I myself prefer it the other way, but here it is. :-)\n\n builtin-shortlog.c |   19 +++----------------\n 1 files changed, 3 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex d03f14f..90e76ae 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -36,7 +36,6 @@ static void insert_one_record(struct shortlog *log,\n \tconst char *dot3 = log->common_repo_prefix;\n \tchar *buffer, *p;\n \tstruct string_list_item *item;\n-\tstruct string_list *onelines;\n \tchar namebuf[1024];\n \tsize_t len;\n \tconst char *eol;\n@@ -68,12 +67,9 @@ static void insert_one_record(struct shortlog *log,\n \t\tsnprintf(namebuf + len, room, \" %.*s\", maillen, boemail);\n \t}\n \n-\tbuffer = xstrdup(namebuf);\n-\titem = string_list_insert(buffer, &log->list);\n+\titem = string_list_insert(namebuf, &log->list);\n \tif (item->util == NULL)\n \t\titem->util = xcalloc(1, sizeof(struct string_list));\n-\telse\n-\t\tfree(buffer);\n \n \t/* Skip any leading whitespace, including any blank lines. */\n \twhile (*oneline && isspace(*oneline))\n@@ -104,16 +100,7 @@ static void insert_one_record(struct shortlog *log,\n \t\t}\n \t}\n \n-\tonelines = item->util;\n-\tif (onelines->nr >= onelines->alloc) {\n-\t\tonelines->alloc = alloc_nr(onelines->nr);\n-\t\tonelines->items = xrealloc(onelines->items,\n-\t\t\t\tonelines->alloc\n-\t\t\t\t* sizeof(struct string_list_item));\n-\t}\n-\n-\tonelines->items[onelines->nr].util = NULL;\n-\tonelines->items[onelines->nr++].string = buffer;\n+\tstring_list_append(buffer, item->util);\n }\n \n static void read_from_stdin(struct shortlog *log)\n@@ -323,7 +310,7 @@ void shortlog_output(struct shortlog *log)\n \t\t}\n \n \t\tonelines->strdup_strings = 1;\n-\t\tstring_list_clear(onelines, 1);\n+\t\tstring_list_clear(onelines, 0);\n \t\tfree(onelines);\n \t\tlog->list.items[i].util = NULL;\n \t}\n-- \n1.6.1.307.g07803\n"},{"id":"98992","messageId":"1230670904-27808-1-git-send-email-dato@net.com.org.es","threadId":"16856","inReplyTo":"1230668722-26394-1-git-send-email-dato@net.com.org.es","subject":"[PATCH v3] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily","fromName":"Adeodato Simó","fromEmail":"dato@net.com.org.es","sentAt":"2008-12-30T21:01:44Z","receivedAt":"2008-12-30T21:01:44Z","isPatch":true,"sender":{"key":"dato@net.com.org.es","avatar":"https://gravatar.com/avatar/952ec7d5d5663eb8baf631b5c37f9c58480a881920dd5f8a2d3a71f969b72b53?d=mp&s=160"},"body":"Make insert_one_record() use string_list_append(), instead of duplicating\nits code. Because of this, do not free the \"util\" member when clearing the\n\"onelines\" string lists: with the new code path it is not initialized to\nany value (was being initialized to NULL previously).\n\nAlso, avoid unnecessary strdup() calls when inserting names in log->list.\nThis list always has \"strdup_strings\" activated, hence strdup'ing namebuf is\nunnecessary. This change also removes a latent memory leak in the old code.\n\nNB: The duplicated code mentioned above predated the appearance of\nstring_list_append().\n\nSigned-off-by: Adeodato Simó <dato@net.com.org.es>\n---\n\nNow with the obvious mistake in the log fixed, apologies. (s/cleanup in//)\n\n builtin-shortlog.c |   19 +++----------------\n 1 files changed, 3 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex d03f14f..90e76ae 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -36,7 +36,6 @@ static void insert_one_record(struct shortlog *log,\n \tconst char *dot3 = log->common_repo_prefix;\n \tchar *buffer, *p;\n \tstruct string_list_item *item;\n-\tstruct string_list *onelines;\n \tchar namebuf[1024];\n \tsize_t len;\n \tconst char *eol;\n@@ -68,12 +67,9 @@ static void insert_one_record(struct shortlog *log,\n \t\tsnprintf(namebuf + len, room, \" %.*s\", maillen, boemail);\n \t}\n \n-\tbuffer = xstrdup(namebuf);\n-\titem = string_list_insert(buffer, &log->list);\n+\titem = string_list_insert(namebuf, &log->list);\n \tif (item->util == NULL)\n \t\titem->util = xcalloc(1, sizeof(struct string_list));\n-\telse\n-\t\tfree(buffer);\n \n \t/* Skip any leading whitespace, including any blank lines. */\n \twhile (*oneline && isspace(*oneline))\n@@ -104,16 +100,7 @@ static void insert_one_record(struct shortlog *log,\n \t\t}\n \t}\n \n-\tonelines = item->util;\n-\tif (onelines->nr >= onelines->alloc) {\n-\t\tonelines->alloc = alloc_nr(onelines->nr);\n-\t\tonelines->items = xrealloc(onelines->items,\n-\t\t\t\tonelines->alloc\n-\t\t\t\t* sizeof(struct string_list_item));\n-\t}\n-\n-\tonelines->items[onelines->nr].util = NULL;\n-\tonelines->items[onelines->nr++].string = buffer;\n+\tstring_list_append(buffer, item->util);\n }\n \n static void read_from_stdin(struct shortlog *log)\n@@ -323,7 +310,7 @@ void shortlog_output(struct shortlog *log)\n \t\t}\n \n \t\tonelines->strdup_strings = 1;\n-\t\tstring_list_clear(onelines, 1);\n+\t\tstring_list_clear(onelines, 0);\n \t\tfree(onelines);\n \t\tlog->list.items[i].util = NULL;\n \t}\n-- \n1.6.1.307.g07803\n"}]}