Re: [PATCH] document string_list_clear

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 9, 2014, 20:00 UTC
Message-ID: <20141209200014.GX16345@google.com>
In-Reply-To: <xmqq7fy0mx70.fsf@gitster.dls.corp.google.com>

Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 23 quoted lines

>>> +/*
>>> + * Clears the string list, so it has zero items. All former items are freed.
>>> + * If free_util is true, all util pointers are also freed.
>>> + */
>>>  void string_list_clear(struct string_list *list, int free_util);
>>
>> The api doc says
>>
>> 	Free a string_list. The `string` pointer of the items will be freed in
>> 	case the `strdup_strings` member of the string_list is set. The second
>> 	parameter controls if the `util` pointer of the items should be freed
>> 	or not.
>>
>> One option here would be to say
>>
>> 	Free a string_list.  See Documentation/technical/api-string-list.txt
>> 	for details.
>
> If we later introduce string_list_free() that is in line with the
> distinction between "free" and "clear" discussed on this thread, the
> comment for this function needs to be fixed to "Clear the string
> list.  See $doc". and that is not much different from "See $doc"
> without the first sentence which is the function name.

I still find the term "clear" to be confusing here. It makes me think the function will be analagous to strbuf_reset, when it's actually analagous to strbuf_release.

In other words, the important thing is that this frees all members of the string_list. There might be a clearer way to say that. The string_list itself may not be dynamically allocated --- I'm not sure what it would mean to free it.

How about string_list_release?

I think I could get used to "clear" if we used it consistently though. I suspect anything we do will be confusing, unless we make them consistent.

> Perhaps the API doc that currently says "Free" is the only thing
> that needs fixing?

I find the API doc to be pretty clear, actually. If someone was confused in practice then I'd be happy to try to debug the wording.

>                     And perhaps add "See $doc" at the beginning of
> the header and remove duplicated comments we already have in the
> file?

Yes, that sounds like a good way to go about it.

Thanks, Jonathan