* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.