On Sat, Aug 2, 2025 at 10:18 AM Christian Couder
<christian.couder@xxxxxxxxx> wrote:
>
> On Fri, Aug 1, 2025 at 7:06 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> > 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@xxxxxxxxx>
> > ---
> > 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
>
> Yeah, I think that could help. Thanks!
For information, I have asked ChatGPT o3-pro (which can analyse a
whole repo) about this and here is what it says:
---------------------------
Q: In the source code of the Git SCM software are there standard ways
of naming functions that release memory from a struct or clear a
struct? Are they consistently named using "clear" or "release" or
"free" or "clear_and_free" or other things like that depending on what
they actually do?
A:
What the names mean in Git’s C code – quick rules of thumb
- *_clear():
effect: Free only the heap‑memory owned by the struct (arrays,
strings, hash‑table buckets, etc.) and reset its fields to an
initial/empty state.
reuse possible: Yes – the variable itself remains valid and can be
filled again.
- *_release():
effect: Historical synonym for the same thing as *_clear(), kept for
backward compatibility in a few low‑level APIs.
reuse possible: Yes – same as above.
- free_*() or *_free():
effect: Free the struct itself (it was malloc‑ed) and everything it
points at. After the call the pointer becomes dangling and must not be
reused.
reuse possible: No – the storage is gone.
- *_clear_and_free():
effect: Convenience wrapper that first calls the type’s *_clear() on
each contained element and then frees those elements themselves.
Mainly used for containers implemented as macros.
reuse possible: Not applicable – it is meant to be the last
operation on the container.
How consistent is the convention?
- Within each mini‑API the convention is very strict
The strbuf API always pairs strbuf_init() / strbuf_release(), the
strvec API always pairs strvec_init() / strvec_clear(), etc.
- Across the whole Git code‑base the spelling is mostly consistent but
not 100 % uniform
Very early core helpers (pre‑1.6): usually *_release()
Code written in the last decade: *_clear() for “empty in‑place”,
free_*() for “destroy”
- No “mixed‑semantics” functions
You will not find a helper that sometimes frees the struct and
sometimes only clears it depending on flags. The name alone tells you
which category it belongs to.
[...]
Bottom line
Git uses two distinct verbs to convey intent:
- clear / release – light‑weight reset, keep the container alive
- free – heavy‑weight destruction of the container itself
The rule is consistent enough that you can rely on it when reading or
writing Git’s C code; the rare exceptions are historical, not
ambiguous.
Q: Some say that if the structure can be immediately reused afterwards
without requiring another reinit the function should be named using
"clear", otherwise if it only releases memory then it should be called
"release". Is it true?
A:
Short answer – no.
In the Git source tree the verbs clear and release are not used to
distinguish “can I reuse the struct afterwards?” vs. “is this the
final teardown?”. Both variants free all heap memory owned by the
structure and then reset the in‑memory object to its pristine, empty
state, so the variable can be used again without an extra call to the
corresponding …_init() helper.
[...]
Why two different verbs exist
- Historical baggage – the very early helper that everybody still
uses, strbuf, was written long before today’s “*_clear()” convention
emerged, so its lifecycle pair is strbuf_init() / strbuf_release().
- Later subsystems standardised on *_clear() when they were introduced
(string‑list, strvec, oidset, hashmap, etc.).
- Because Git’s internal APIs are used by thousands of call‑sites,
there has been no large‑scale renaming; both spellings therefore
coexist.
[...]
Conclusion
The folk rule you quoted (“* use clear if the struct can be reused,
release if not *”) does not match what the Git code actually does:
strbuf_release() allows immediate reuse – just like every *_clear() helper does.
The difference in naming is historical, not semantic.
If you want to know whether the variable itself has been freed, look
for free_*()/*_free(), not for the presence or absence of the word
release.
Q: Do you have other examples than strbuf_release using "release"
where the function behaves like a "clear" function?
A:
Yes – a handful of other helpers are spelled “*_release()”, yet they
merely clear the inside of an on‑stack/object‑lifetime struct and
leave the container itself reusable
- Line reader: void line_buffer_release(struct line_buffer *buf);
Closes the underlying file descriptor and frees its internal scratch
space; the struct line_buffer itself remains valid and can be
re‑initialised with line_buffer_open() if desired.
- Packet‑line writer: void packet_writer_release(struct packet_writer *writer);
Calls strbuf_release(&writer‑>scratch_buf) and clears a few flags;
the struct packet_writer lives on so the caller may point it at
another FILE * later.
- FS‑monitor IPC channel: void fsmonitor_ipc__release(struct
fsmonitor_ipc *ipc);
Shuts down the socket, frees small temp buffers, and zeroes the
struct for re‑use inside the long‑running daemon.
All of these match the behaviour of strbuf_release() rather than that
of the many *_free() helpers which actually free() the struct itself.
How many are there?
A quick git grep '_release(struct' over the Git 2.50 tree finds fewer
than a dozen hits, and every one of them follows this
“clear‑but‑don’t‑free” pattern. In other words:
- *_release() in Git never deallocates the container itself.
- The form survives today only in subsystems that pre‑date the later
project‑wide switch to *_clear() (or that wrap strbuf_release()
internally).
- Newer code consistently uses *_clear() for the same semantics.
Bottom line
strbuf_release() is not unique, but it is the template that the other
rare *_release() helpers copy: they wipe the contents and reset the
struct so you can keep using the variable. If you are looking at Git C
code and see “_release” in a function name, you can safely assume
“clear in place” – not “free the object.”
---------------------------
In short it looks like it doesn't think there should be a distinction
between *_release() and *_clear(), because in many cases the existing
*_release() functions (starting with strbuf_release()) leave the
struct ready for immediate reuse.
For the documentation update, it suggests:
- `S_release()` releases a structure's contents without freeing the
structure. In older subsystems (e.g. *strbuf*), the helper also
resets the object to its initial, empty state; newer code should
prefer `S_clear()` for that purpose.
But if our intention is to tend towards a clear distinction between
"clear" and "release" even if in practice there is not a clear
distinction right now (because of historical reasons), I think we
could compromise with something like:
- `S_release()` releases a structure's contents without reinitializing
the structure for immediate reuse, and without freeing the structure
itself. In older subsystems (e.g. *strbuf*), the helper also
resets the object to its initial, empty state; newer code should
prefer `S_clear()` for that purpose.
