Subject: Re: [PATCH v3 0/5] date.[ch] API: split from cache.h, add API docs, stop leaking memory

From: Junio C Hamano <[email-masked]>
Date: Wed, 16 Feb 2022 09:45:07 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> 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.
>     +

If this step were to include the new header in "cache.h" to reduce
the patch noise, and there were a follow-up step to update the *.c
files to include the new header while removing the inclusion of the
header from "cache.h", then the above would make a fine draft for
the log message that justifies that follow-up step.

But if we are doing these two things in a single step, the paragraph
would not make a very useful comment to help readers of "git log".

> 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);

OK.

> 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.

Ah, it is not even a leak, then.  Is blame the only thing that uses
parse_date_format() outside the revision walkers?

Thanks.