Re: [PATCH] document string_list_clear

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

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