Re: [PATCH v3 0/5] date.[ch] API: split from cache.h, add API docs, stop leaking memory
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 16, 2022, 17:45 UTC
- Message-ID
- <xmqqr182u2n0.fsf@gitster.g>
- In-Reply-To
- <cover-v3-0.5-00000000000-20220216T081203Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 11 quoted lines
> 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".
Show 12 quoted lines
> 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.
Show 9 quoted lines
> 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.