Re: [PATCH] document string_list_clear

[Jeff King <peff@peff.net>]

On Fri, Dec 05, 2014 at 06:04:58PM -0800, Jonathan Nieder wrote:

> That reminds me: why do we call this string_list_clear instead of
> string_list_free?

Because it does not free the struct itself, and because you can then use
the result again. I think we try to draw a distinction between:

  /* Free resources, but do not reinitialize! */
  void string_list_free_data(struct string_list *s)
  {
	int i;
	for (i = 0; i < s->nr; i++)
		free(s->items[i].string);
	free(s->items[i];
  }

  /* Free resources, and go back to initialized but empty state */
  void string_list_clear(struct string_list *s)
  {
	string_list_free_data(s);
	s->nr = s->alloc = 0;
	s->items = NULL;
  }

  /* Free all resources from dynamically allocated structure */
  void string_list_free(struct string_list *s)
  {
	string_list_clear(s);
	free(s);
  }

Ideally we use consistent names to distinguish between them. We are not
always consistent, though (probably strbuf_release should be called
strbuf_clear for consistency). But I think we are fairly consistent that
"_free()" means "...and free the pointer, too".

In general, we try to avoid the first as a public interface (because it
is error-prone when somebody tries to reuse the list, and the extra work
of zero-ing the pointer is not enough to care about).

We also tend to avoid the third, because it is quite often not the
business of the object whether it was dynamically constructed or not
(exceptions tend to be node-oriented structures like linked lists and
trees; c.f. cache_tree and commit_list).

Maybe that should go into CodingGuidelines? It's not really style,
exactly, but it is convention.

-Peff