{"thread":{"id":"60122","subject":"[PATCH] diff: --dirstat leakfix","startedAt":"2023-08-17T21:16:21Z","lastAt":"2023-08-17T21:16:21Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"480767","messageId":"xmqqbkf5bcbl.fsf@gitster.g","threadId":"60122","inReplyTo":null,"subject":"[PATCH] diff: --dirstat leakfix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-17T21:15:10Z","receivedAt":"2023-08-17T21:16:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The dirstat_dir structure holds a list of files that had \"damages\"\nand is used to summarize the change by directory.  It, specifically\nits .files member, was allocated, used, and then left behind,\nleaking.\n\nWhat is tricky is that dir.files[] array is allocated and walked by\nnot pointing into the array with an incrementing offset, but by\nincrementing the beginning of the array, so we need to remember the\noriginal address of dir.files[] array before letting gather_dirstat()\nwalk over it, and free the original address once we are done.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c                  | 14 ++++++++++++--\n t/t4047-diff-dirstat.sh |  2 ++\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git c/diff.c w/diff.c\nindex 648f6717a5..03d0cfc700 100644\n--- c/diff.c\n+++ w/diff.c\n@@ -2977,6 +2977,7 @@ static void show_dirstat(struct diff_options *options)\n \tunsigned long changed;\n \tstruct dirstat_dir dir;\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tstruct dirstat_file *to_free;\n \n \tdir.files = NULL;\n \tdir.alloc = 0;\n@@ -3060,13 +3061,17 @@ static void show_dirstat(struct diff_options *options)\n \t\tdir.nr++;\n \t}\n \n+\tto_free = dir.files;\n+\n \t/* This can happen even with many files, if everything was renames */\n \tif (!changed)\n-\t\treturn;\n+\t\tgoto free_return;\n \n \t/* Show all directories with more than x% of the changes */\n \tQSORT(dir.files, dir.nr, dirstat_compare);\n \tgather_dirstat(options, &dir, changed, \"\", 0);\n+free_return:\n+\tfree(to_free);\n }\n \n static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *options)\n@@ -3074,6 +3079,7 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o\n \tint i;\n \tunsigned long changed;\n \tstruct dirstat_dir dir;\n+\tstruct dirstat_file *to_free;\n \n \tif (data->nr == 0)\n \t\treturn;\n@@ -3104,13 +3110,17 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o\n \t\tdir.nr++;\n \t}\n \n+\tto_free = dir.files;\n+\n \t/* This can happen even with many files, if everything was renames */\n \tif (!changed)\n-\t\treturn;\n+\t\tgoto free_return;\n \n \t/* Show all directories with more than x% of the changes */\n \tQSORT(dir.files, dir.nr, dirstat_compare);\n \tgather_dirstat(options, &dir, changed, \"\", 0);\n+free_return:\n+\tfree(to_free);\n }\n \n static void free_diffstat_file(struct diffstat_file *f)\ndiff --git c/t/t4047-diff-dirstat.sh w/t/t4047-diff-dirstat.sh\nindex 7fec2cb9cd..70224c3da1 100755\n--- c/t/t4047-diff-dirstat.sh\n+++ w/t/t4047-diff-dirstat.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='diff --dirstat tests'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # set up two commits where the second commit has these files\n"}]}