* [PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code @ 2008-12-24 16:34 Adeodato Simó 2008-12-30 12:20 ` Johannes Schindelin 0 siblings, 1 reply; 4+ messages in thread From: Adeodato Simó @ 2008-12-24 16:34 UTC (permalink / raw) To: git, gitster; +Cc: Adeodato Simó Also, when clearing the "onelines" string lists, do not free the "util" member: with string_list_append() is not initialized to any value (and was being initialized to NULL previously anyway). NB: The duplicated code in builtin-shortlog.c predated the appearance of string_list_append(). Signed-off-by: Adeodato Simó <dato@net.com.org.es> --- builtin-shortlog.c | 14 ++------------ 1 files changed, 2 insertions(+), 12 deletions(-) diff --git a/builtin-shortlog.c b/builtin-shortlog.c index d03f14f..4c5d761 100644 --- a/builtin-shortlog.c +++ b/builtin-shortlog.c @@ -36,7 +36,6 @@ static void insert_one_record(struct shortlog *log, const char *dot3 = log->common_repo_prefix; char *buffer, *p; struct string_list_item *item; - struct string_list *onelines; char namebuf[1024]; size_t len; const char *eol; @@ -104,16 +103,7 @@ static void insert_one_record(struct shortlog *log, } } - onelines = item->util; - if (onelines->nr >= onelines->alloc) { - onelines->alloc = alloc_nr(onelines->nr); - onelines->items = xrealloc(onelines->items, - onelines->alloc - * sizeof(struct string_list_item)); - } - - onelines->items[onelines->nr].util = NULL; - onelines->items[onelines->nr++].string = buffer; + string_list_append(buffer, item->util); } static void read_from_stdin(struct shortlog *log) @@ -323,7 +313,7 @@ void shortlog_output(struct shortlog *log) } onelines->strdup_strings = 1; - string_list_clear(onelines, 1); + string_list_clear(onelines, 0); free(onelines); log->list.items[i].util = NULL; } -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code 2008-12-24 16:34 [PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code Adeodato Simó @ 2008-12-30 12:20 ` Johannes Schindelin 2008-12-30 20:25 ` [PATCH v2] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily Adeodato Simó 0 siblings, 1 reply; 4+ messages in thread From: Johannes Schindelin @ 2008-12-30 12:20 UTC (permalink / raw) To: Adeodato Simó; +Cc: git, gitster [-- Attachment #1: Type: TEXT/PLAIN, Size: 557 bytes --] Hi, On Wed, 24 Dec 2008, Adeodato Simó wrote: > Also, when clearing the "onelines" string lists, do not free the "util" > member: with string_list_append() is not initialized to any value (and > was being initialized to NULL previously anyway). > > NB: The duplicated code in builtin-shortlog.c predated the appearance of > string_list_append(). > > Signed-off-by: Adeodato Simó <dato@net.com.org.es> FWIW I like the patch, but would like it even more if the strdup() removal was squashed in (with an explanation in the commit message). Ciao, Dscho ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily 2008-12-30 12:20 ` Johannes Schindelin @ 2008-12-30 20:25 ` Adeodato Simó 2008-12-30 21:01 ` [PATCH v3] " Adeodato Simó 0 siblings, 1 reply; 4+ messages in thread From: Adeodato Simó @ 2008-12-30 20:25 UTC (permalink / raw) To: git, gitster; +Cc: Johannes Schindelin, Adeodato Simó Make cleanup in insert_one_record() use string_list_append(), instead of duplicating its code. Because of this, do not free the "util" member when clearing the "onelines" string lists: with the new code path it is not initialized to any value (was being initialized to NULL previously). Also, avoid unnecessary strdup() calls when inserting names in log->list. This list always has "strdup_strings" activated, hence strdup'ing namebuf is unnecessary. This change also removes a latent memory leak in the old code. NB: The duplicated code mentioned above predated the appearance of string_list_append(). Signed-off-by: Adeodato Simó <dato@net.com.org.es> --- > FWIW I like the patch, but would like it even more if the strdup() removal > was squashed in (with an explanation in the commit message). Ok, I myself prefer it the other way, but here it is. :-) builtin-shortlog.c | 19 +++---------------- 1 files changed, 3 insertions(+), 16 deletions(-) diff --git a/builtin-shortlog.c b/builtin-shortlog.c index d03f14f..90e76ae 100644 --- a/builtin-shortlog.c +++ b/builtin-shortlog.c @@ -36,7 +36,6 @@ static void insert_one_record(struct shortlog *log, const char *dot3 = log->common_repo_prefix; char *buffer, *p; struct string_list_item *item; - struct string_list *onelines; char namebuf[1024]; size_t len; const char *eol; @@ -68,12 +67,9 @@ static void insert_one_record(struct shortlog *log, snprintf(namebuf + len, room, " %.*s", maillen, boemail); } - buffer = xstrdup(namebuf); - item = string_list_insert(buffer, &log->list); + item = string_list_insert(namebuf, &log->list); if (item->util == NULL) item->util = xcalloc(1, sizeof(struct string_list)); - else - free(buffer); /* Skip any leading whitespace, including any blank lines. */ while (*oneline && isspace(*oneline)) @@ -104,16 +100,7 @@ static void insert_one_record(struct shortlog *log, } } - onelines = item->util; - if (onelines->nr >= onelines->alloc) { - onelines->alloc = alloc_nr(onelines->nr); - onelines->items = xrealloc(onelines->items, - onelines->alloc - * sizeof(struct string_list_item)); - } - - onelines->items[onelines->nr].util = NULL; - onelines->items[onelines->nr++].string = buffer; + string_list_append(buffer, item->util); } static void read_from_stdin(struct shortlog *log) @@ -323,7 +310,7 @@ void shortlog_output(struct shortlog *log) } onelines->strdup_strings = 1; - string_list_clear(onelines, 1); + string_list_clear(onelines, 0); free(onelines); log->list.items[i].util = NULL; } -- 1.6.1.307.g07803 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v3] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily 2008-12-30 20:25 ` [PATCH v2] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily Adeodato Simó @ 2008-12-30 21:01 ` Adeodato Simó 0 siblings, 0 replies; 4+ messages in thread From: Adeodato Simó @ 2008-12-30 21:01 UTC (permalink / raw) To: git, gitster; +Cc: Johannes Schindelin, Adeodato Simó Make insert_one_record() use string_list_append(), instead of duplicating its code. Because of this, do not free the "util" member when clearing the "onelines" string lists: with the new code path it is not initialized to any value (was being initialized to NULL previously). Also, avoid unnecessary strdup() calls when inserting names in log->list. This list always has "strdup_strings" activated, hence strdup'ing namebuf is unnecessary. This change also removes a latent memory leak in the old code. NB: The duplicated code mentioned above predated the appearance of string_list_append(). Signed-off-by: Adeodato Simó <dato@net.com.org.es> --- Now with the obvious mistake in the log fixed, apologies. (s/cleanup in//) builtin-shortlog.c | 19 +++---------------- 1 files changed, 3 insertions(+), 16 deletions(-) diff --git a/builtin-shortlog.c b/builtin-shortlog.c index d03f14f..90e76ae 100644 --- a/builtin-shortlog.c +++ b/builtin-shortlog.c @@ -36,7 +36,6 @@ static void insert_one_record(struct shortlog *log, const char *dot3 = log->common_repo_prefix; char *buffer, *p; struct string_list_item *item; - struct string_list *onelines; char namebuf[1024]; size_t len; const char *eol; @@ -68,12 +67,9 @@ static void insert_one_record(struct shortlog *log, snprintf(namebuf + len, room, " %.*s", maillen, boemail); } - buffer = xstrdup(namebuf); - item = string_list_insert(buffer, &log->list); + item = string_list_insert(namebuf, &log->list); if (item->util == NULL) item->util = xcalloc(1, sizeof(struct string_list)); - else - free(buffer); /* Skip any leading whitespace, including any blank lines. */ while (*oneline && isspace(*oneline)) @@ -104,16 +100,7 @@ static void insert_one_record(struct shortlog *log, } } - onelines = item->util; - if (onelines->nr >= onelines->alloc) { - onelines->alloc = alloc_nr(onelines->nr); - onelines->items = xrealloc(onelines->items, - onelines->alloc - * sizeof(struct string_list_item)); - } - - onelines->items[onelines->nr].util = NULL; - onelines->items[onelines->nr++].string = buffer; + string_list_append(buffer, item->util); } static void read_from_stdin(struct shortlog *log) @@ -323,7 +310,7 @@ void shortlog_output(struct shortlog *log) } onelines->strdup_strings = 1; - string_list_clear(onelines, 1); + string_list_clear(onelines, 0); free(onelines); log->list.items[i].util = NULL; } -- 1.6.1.307.g07803 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-12-30 21:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-12-24 16:34 [PATCH] builtin-shortlog.c: use string_list_append() instead of duplicating its code Adeodato Simó 2008-12-30 12:20 ` Johannes Schindelin 2008-12-30 20:25 ` [PATCH v2] builtin-shortlog.c: use string_list_append(), and don't strdup unnecessarily Adeodato Simó 2008-12-30 21:01 ` [PATCH v3] " Adeodato Simó
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).