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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]