On Wed, Jun 06 2018, Derrick Stolee wrote:
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> builtin/commit-graph.c | 39 +++++++++++++--------------------------
> commit-graph.c | 15 +++++++--------
> commit-graph.h | 7 +++----
> 3 files changed, 23 insertions(+), 38 deletions(-)
> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
> index 3079cde6f9..d8eb8278b3 100644
> --- a/builtin/commit-graph.c
> +++ b/builtin/commit-graph.c
> @@ -118,13 +118,9 @@ static int graph_read(int argc, const char **argv)
>
> static int graph_write(int argc, const char **argv)
> {
> - const char **pack_indexes = NULL;
> - int packs_nr = 0;
> - const char **commit_hex = NULL;
> - int commits_nr = 0;
> - const char **lines = NULL;
> - int lines_nr = 0;
> - int lines_alloc = 0;
> + struct string_list *pack_indexes = NULL;
> + struct string_list *commit_hex = NULL;
> + struct string_list lines;
>
> static struct option builtin_commit_graph_write_options[] = {
> OPT_STRING(0, "object-dir", &opts.obj_dir,
> @@ -150,32 +146,23 @@ static int graph_write(int argc, const char **argv)
>
> if (opts.stdin_packs || opts.stdin_commits) {
> struct strbuf buf = STRBUF_INIT;
> - lines_nr = 0;
> - lines_alloc = 128;
> - ALLOC_ARRAY(lines, lines_alloc);
> -
> - while (strbuf_getline(&buf, stdin) != EOF) {
> - ALLOC_GROW(lines, lines_nr + 1, lines_alloc);
> - lines[lines_nr++] = strbuf_detach(&buf, NULL);
> - }
> -
> - if (opts.stdin_packs) {
> - pack_indexes = lines;
> - packs_nr = lines_nr;
> - }
> - if (opts.stdin_commits) {
> - commit_hex = lines;
> - commits_nr = lines_nr;
> - }
> + string_list_init(&lines, 0);
> +
> + while (strbuf_getline(&buf, stdin) != EOF)
> + string_list_append(&lines, strbuf_detach(&buf, NULL));
> +
> + if (opts.stdin_packs)
> + pack_indexes = &lines;
> + if (opts.stdin_commits)
> + commit_hex = &lines;
> }
>
> write_commit_graph(opts.obj_dir,
> pack_indexes,
> - packs_nr,
> commit_hex,
> - commits_nr,
> opts.append);
>
> + string_list_clear(&lines, 0);
> return 0;
> }
This results in an invalid free() & segfault because you're freeing
&lines which may not have been allocated by string_list_init().
Monkeypatch on top which I used to fix it:
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 76423b3fa5..c7eb68aa3a 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -122,6 +122,7 @@ static int graph_write(int argc, const char **argv)
struct string_list *pack_indexes = NULL;
struct string_list *commit_hex = NULL;
struct string_list lines;
+ int free_lines = 0;
static struct option builtin_commit_graph_write_options[] = {
OPT_STRING(0, "object-dir", &opts.obj_dir,
@@ -155,6 +156,7 @@ static int graph_write(int argc, const char **argv)
if (opts.stdin_packs || opts.stdin_commits) {
struct strbuf buf = STRBUF_INIT;
string_list_init(&lines, 0);
+ free_lines = 1;
while (strbuf_getline(&buf, stdin) != EOF)
string_list_append(&lines, strbuf_detach(&buf, NULL));
@@ -170,7 +172,8 @@ static int graph_write(int argc, const char **argv)
commit_hex,
opts.append);
- string_list_clear(&lines, 0);
+ if (free_lines)
+ string_list_clear(&lines, 0);
return 0;
}
But probably having a pointer to the struct which is NULL etc. is
better.
