Re: [PATCH v6 1/4] last-modified: new subcommand to show when files were last modified
On Fri, Aug 01, 2025 at 10:06:55AM -0700, Junio C Hamano wrote:
Show 50 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
>
> >> > +static void last_modified_release(struct last_modified *lm)
> >>
> >> I think these days we tend to name those functions using "clear"
> >> instead of "release"
> >
> > It actually depends: if the structure can be immediately reused
> > afterwards without requiring another reinit it would be caller "clear"
> > indeed. On the other hand, if we only release memory it's "release".
> >
> > I think this function here falls into the latter category, so it's
> > correctly named.
>
> Given that even a long-time contributor gets confused (including me,
> who needed to see where we documented this for our developers),
> perhaps a clarification patch is in order?
>
> --- >8 ---
> Subject: CodingGuidelines: clarify that S_release() does not reinitialize
>
> In the section for naming various API functions, the fact that
> S_release() only releases the resources without preparing the
> structure for immediate reuse becomes only apparent when you
> readentries for S_release() and S_clear().
>
> Clarify the description of S_release() a bit to make the entry self
> sufficient.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/CodingGuidelines | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
> index c1046abfb7..76ec6268f2 100644
> --- c/Documentation/CodingGuidelines
> +++ w/Documentation/CodingGuidelines
> @@ -610,8 +610,9 @@ For C programs:
> - `S_init()` initializes a structure without allocating the
> structure itself.
>
> - - `S_release()` releases a structure's contents without freeing the
> - structure.
> + - `S_release()` releases a structure's contents without reinitializing
> + the structure for immediate reuse, and without freeing the structure
> + itself.
>
> - `S_clear()` is equivalent to `S_release()` followed by `S_init()`
> such that the structure is directly usable after clearing it. When
Yup, this looks like a reasonable change to me, thanks!
Patrick