[PATCH v3 0/5] date.[ch] API: split from cache.h, add API docs, stop leaking memory
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 16, 2022, 08:14 UTC
- Message-ID
- <cover-v3-0.5-00000000000-20220216T081203Z-avarab@gmail.com>
- In-Reply-To
- <cover-v2-0.5-00000000000-20220204T235143Z-avarab@gmail.com>
Fix memory leaks in the date.[ch] API, in preparation for larger changes to make the revision walking API stop leaking memory.
This is small re-roll of v2 to address Junio's feedback on that version. For v2 see: https://lore.kernel.org/git/cover-v2-0.5-00000000000-20220204T235143Z-avarab@gmail.com/
This is a documentation and commit-message only update. As explained below I think it makes sense to punt on the "builtin/blame.c" leak, and to keep 2/5 as-is with date.h not included in cache.h, but those things are now all rationalized in the commit message. Thanks for the review Junio!
Ævar Arnfjörð Bjarmason (5): cache.h: remove always unused show_date_human() declaration date API: create a date.h, split from cache.h date API: provide and use a DATE_MODE_INIT date API: add basic API docs date API: add and use a date_mode_release()
archive-zip.c | 1 + builtin/am.c | 1 + builtin/commit.c | 1 + builtin/fast-import.c | 1 + builtin/show-branch.c | 1 + builtin/tag.c | 1 + cache.h | 50 ----------------------------- config.c | 1 + date.c | 9 ++++-- date.h | 74 +++++++++++++++++++++++++++++++++++++++++++ http-backend.c | 1 + ident.c | 1 + object-name.c | 1 + pretty.h | 10 ++++++ ref-filter.c | 3 +- reflog-walk.h | 1 + refs.c | 1 + strbuf.c | 1 + t/helper/test-date.c | 5 ++- t/t0006-date.sh | 2 ++ 20 files changed, 112 insertions(+), 54 deletions(-) create mode 100644 date.h
Range-diff against v2:
1: fb21bd7b2c5 = 1: 97746d97810 cache.h: remove always unused show_date_human() declaration
2: 96c904d0b9a ! 2: f73aa601e95 date API: create a date.h, split from cache.h
@@ Commit message
use the "DATE_MODE()" macro we now define in date.h, let's have them
include it.
+ We could simply include this new header in "cache.h", but as this
+ change shows these functions weren't common enough to warrant
+ including in it in the first place. By moving them out of cache.h
+ changes to this API will no longer cause a (mostly) full re-build of
+ the project when "make" is run.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## archive-zip.c ##
3: 9ef003a83bd = 3: 764147e90e1 date API: provide and use a DATE_MODE_INIT
4: 3f70b1aa4c5 ! 4: 5c244960133 date API: add basic API docs
@@ date.h: struct date_mode {
struct date_mode *date_mode_from_type(enum date_mode_type type);
+/**
-+ * Show the date given an initialized "struct date_mode" (usually from
-+ * the DATE_MODE() macro).
++ * Format <'time', 'timezone'> into static memory according to 'mode'
++ * and return it. The mode is an initialized "struct date_mode"
++ * (usually from the DATE_MODE() macro).
+ */
const char *show_date(timestamp_t time, int timezone, const struct date_mode *mode);
+
5: 60dbadacb16 ! 5: b1ee9a30913 date API: add and use a date_mode_release()
@@ Commit message
release_revisions() in "revision.c", as they have to do with leaks via
"struct rev_info".
+ There is also a leak in "builtin/blame.c" due to its call to
+ parse_date_format() to parse the "blame.date" configuration. However
+ as it declares a file-level "static struct date_mode blame_date_mode"
+ to track the data, LSAN will not report it as a leak. It's possible to
+ get valgrind(1) to complain about it with e.g.:
+
+ valgrind --leak-check=full --show-leak-kinds=all ./git -P -c blame.date=format:%Y blame README.md
+
+ But let's focus on things LSAN complains about, and are thus
+ observable with "TEST_PASSES_SANITIZE_LEAK=true". We should get to
+ fixing memory leaks in "builtin/blame.c", but as doing so would
+ require some re-arrangement of cmd_blame() let's leave it for some
+ other time.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## date.c ##-- 2.35.1.1028.g2d2d4be19de