threads / patch / 24994

v2string-list: Document STRING_LIST_INIT_* macros.

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

## tl;dr

3 messages between Sep 5, 2010 and Sep 6, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Thiago Farina· Sep 5, 2010, 23:40 UTC · lore

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(-)
Show changes to Documentation/technical/api-string-list.txt +21 −3
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
Jonathan Nieder· Sep 5, 2010, 23:57 UTC · re: Thiago Farina · lore

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

Thiago Farina wrote:
Show 16 quoted lines
> +++ 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.

Show 10 quoted lines
> @@ -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

Thiago Farina· Sep 6, 2010, 00:15 UTC · re: Jonathan Nieder · lore

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

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.
Show 12 quoted lines
>> @@ -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.

← back to recent threads