[PATCH v2] string-list: Document STRING_LIST_INIT_* macros.

[PATCH v2] string-list: Document STRING_LIST_INIT_* macros.

[Thiago Farina]

Clarify the modern ways to initialize a string_list. Text roughly
based on the analogous passage from api-strbuf.txt.

(Note: Based on the demo patch of Jonathan Nieder).

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 Documentation/technical/api-string-list.txt |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt
index 3f575bd..1510ee2 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -11,7 +11,14 @@ The caller:
 
 . Allocates and clears a `struct string_list` variable.
 
-. Initializes the members. You might want to set the flag `strdup_strings`
+. Initializes the members. A string_list has to by `= STRING_LIST_INT_NODUP` or
+  `= STRING_LIST_INIT_DUP` before it can be used.
+  Strings in lists initialized with the _DUP variant will be
+  automatically strdup()ed on insertion and free()ed on removal.
+  For example, this is necessary when you add something like
+  git_path("..."), since that function returns a static buffer
+  that will change with the next call to git_path().
+
   if the strings should be strdup()ed. For example, this is necessary
   when you add something like git_path("..."), since that function returns
   a static buffer that will change with the next call to git_path().
@@ -34,10 +41,9 @@ member (you need this if you add things later) and you should set the
 Example:
 
 ----
-struct string_list list;
+struct string_list list = STRING_LIST_DUP;
 int i;
 
-memset(&list, 0, sizeof(struct string_list));
 string_list_append(&list, "foo");
 string_list_append(&list, "bar");
 for (i = 0; i < list.nr; i++)
@@ -52,6 +58,18 @@ However, if you use the list to check if a certain string was added
 already, you should not do that (using unsorted_string_list_has_string()),
 because the complexity would be quadratic again (but with a worse factor).
 
+Macros
+------
+
+`STRING_LIST_INIT_NODUP`::
+
+	Initialize the members and set the `strdup_strings` member to 0.
+
+`STRING_LIST_INIT_DUP`::
+
+	Initialize the members and set the `strdup_strings` member to 1.
+
+
 Functions
 ---------
 
-- 
1.7.2.3.313.gcd15

Re: [PATCH v2] string-list: Document STRING_LIST_INIT_* macros.

[Jonathan Nieder]

Thiago Farina wrote:

> +++ b/Documentation/technical/api-string-list.txt
> @@ -11,7 +11,14 @@ The caller:
>  
>  . Allocates and clears a `struct string_list` variable.
>  
> -. Initializes the members. You might want to set the flag `strdup_strings`
> +. Initializes the members. A string_list has to by `= STRING_LIST_INT_NODUP` or
> +  `= STRING_LIST_INIT_DUP` before it can be used.
> +  Strings in lists initialized with the _DUP variant will be
> +  automatically strdup()ed on insertion and free()ed on removal.
> +  For example, this is necessary when you add something like
> +  git_path("..."), since that function returns a static buffer
> +  that will change with the next call to git_path().
> +
>    if the strings should be strdup()ed. For example, this is necessary
>    when you add something like git_path("...")

Hmm, something seems to have go wrong here.  Did you intend to
insert that line break and duplicate the text below it?

We cannot really say "a string_list has to be initialized with
STRING_LIST_INIT_NODUP or STRING_LIST_INIT_DUP before it is
used" because not all string_lists are statically allocated.
Some (many) are allocated on the heap, which is part of why I
introduced a string_list_init() function in the rfc series.

> @@ -52,6 +58,18 @@ However, if you use the list to check if a certain string was added
>  already, you should not do that (using unsorted_string_list_has_string()),
>  because the complexity would be quadratic again (but with a worse factor).
>  
> +Macros
> +------
> +
> +`STRING_LIST_INIT_NODUP`::
> +
> +	Initialize the members and set the `strdup_strings` member to 0.

I think this section is not very useful.  I think what is intended is
something like

`STRING_LIST_INIT_NODUP`::

	initializer for a string_list with 0 items and the `strdup_strings`
	member equal to 0

but string-list.h says that already:

 struct string_list
 {
	struct string_list_item *items;
	unsigned int nr, alloc;
	unsigned int strdup_strings:1;
 };

 #define STRING_LIST_INIT_NODUP	{ NULL, 0, 0, 0 }

In generally it is not always a good idea to immediately document
every identifier in a project like this one: writing good
documentation takes some time, and maintaining it takes even more, so
when the code already explains something, it tends to be more useful
to document from another angle.

Hope that helps,
Jonathan

Re: [PATCH v2] string-list: Document STRING_LIST_INIT_* macros.

[Thiago Farina]

On Sun, Sep 5, 2010 at 8:57 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hmm, something seems to have go wrong here.  Did you intend to
> insert that line break and duplicate the text below it?
>
Ops, good catch. Fixed that.

>> @@ -52,6 +58,18 @@ However, if you use the list to check if a certain string was added
>>  already, you should not do that (using unsorted_string_list_has_string()),
>>  because the complexity would be quadratic again (but with a worse factor).
>>
>> +Macros
>> +------
>> +
>> +`STRING_LIST_INIT_NODUP`::
>> +
>> +     Initialize the members and set the `strdup_strings` member to 0.
>
> I think this section is not very useful.

OK, removed that section.