threads / patch / 46798

patch, 2 partsTwo small comment fixes.

Subject: [PATCH 0/2] Two small comment fixes.

## tl;dr

5 messages between Sep 21, 2017 and Sep 21, 2017. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Han-Wen Nienhuys· Sep 21, 2017, 12:43 UTC · lore

Han-Wen Nienhuys (2):
  Fix typo in submodule.h
  Document the string_list structure

 string-list.h | 6 ++++++
 submodule.h   | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 2.14.1.821.g8fa685d3b7-goog

Han-Wen Nienhuys· Sep 21, 2017, 12:43 UTC · re: Han-Wen Nienhuys · lore

[PATCH 1/2] Fix typo in submodule.h

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 submodule.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to submodule.h +1 −1

diff --git a/submodule.h b/submodule.h
index 6b52133c8..f0da0277a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -120,7 +120,7 @@ extern int submodule_move_head(const char *path,
 
 /*
  * Prepare the "env_array" parameter of a "struct child_process" for executing
- * a submodule by clearing any repo-specific envirionment variables, but
+ * a submodule by clearing any repo-specific environment variables, but
  * retaining any config in the environment.
  */
 extern void prepare_submodule_repo_env(struct argv_array *out);

-- 
2.14.1.821.g8fa685d3b7-goog

Han-Wen Nienhuys· Sep 21, 2017, 12:43 UTC · re: Han-Wen Nienhuys · lore

[PATCH 2/2] Document the string_list structure

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 string-list.h | 6 ++++++
 1 file changed, 6 insertions(+)

Show changes to string-list.h +6 −0

diff --git a/string-list.h b/string-list.h
index 29bfb7ae4..08b534166 100644
--- a/string-list.h
+++ b/string-list.h
@@ -8,6 +8,12 @@ struct string_list_item {
 
 typedef int (*compare_strings_fn)(const char *, const char *);
 
+/* A resizable array of strings. The strings are owned if
+ * 'strdup_strings' is set. It can be used as a sorted array, and a
+ * custom comparison may be given in 'cmp'. The field 'items[i].util'
+ * may be used to implement an array of pairs. In that case, the
+ * caller is responsible for managing memory pointed to by 'util'.
+ */
 struct string_list {
 	struct string_list_item *items;
 	unsigned int nr, alloc;

-- 
2.14.1.821.g8fa685d3b7-goog

Jeff King· Sep 21, 2017, 16:24 UTC · re: Han-Wen Nienhuys · lore

Re: [PATCH 2/2] Document the string_list structure

On Thu, Sep 21, 2017 at 02:43:38PM +0200, Han-Wen Nienhuys wrote:

Show 22 quoted lines

> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  string-list.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/string-list.h b/string-list.h
> index 29bfb7ae4..08b534166 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -8,6 +8,12 @@ struct string_list_item {
>  
>  typedef int (*compare_strings_fn)(const char *, const char *);
>  
> +/* A resizable array of strings. The strings are owned if
> + * 'strdup_strings' is set. It can be used as a sorted array, and a
> + * custom comparison may be given in 'cmp'. The field 'items[i].util'
> + * may be used to implement an array of pairs. In that case, the
> + * caller is responsible for managing memory pointed to by 'util'.
> + */
>  struct string_list {
>  	struct string_list_item *items;
>  	unsigned int nr, alloc;

There's a considerable amount of documentation for string-list in Documentation/technical/api-string-list.txt.

Perhaps we should look at migrating it into string-list.h, where it's more likely to be found (and kept up to date). We did something similar for strbuf.h a while back, and I think the result is much better. See the commits in:

  git log bdfdaa4978^..d468fa2721

for an example.

-Peff

Brandon Williams· Sep 21, 2017, 17:52 UTC · re: Han-Wen Nienhuys · lore

Re: [PATCH 2/2] Document the string_list structure

On 09/21, Han-Wen Nienhuys wrote:

Show 14 quoted lines

> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  string-list.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/string-list.h b/string-list.h
> index 29bfb7ae4..08b534166 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -8,6 +8,12 @@ struct string_list_item {
>  
>  typedef int (*compare_strings_fn)(const char *, const char *);
>  
> +/* A resizable array of strings. The strings are owned if

nit: the start of the comment block should be on its own line:

  /*
   * A resizable ...

Show 5 quoted lines

> + * 'strdup_strings' is set. It can be used as a sorted array, and a
> + * custom comparison may be given in 'cmp'. The field 'items[i].util'
> + * may be used to implement an array of pairs. In that case, the
> + * caller is responsible for managing memory pointed to by 'util'.
> + */

The util field can be freed if 'free_util' is set when calling string_list_clear() but yeah, in general the caller is managing that memory especially if a complex free function is required (in the case of allocating a struct with dynamically allocated fields to be stored in the util feild).

Just pointing that out, and I'm happy with this and the previous patch.

Show 6 quoted lines

>  struct string_list {
>  	struct string_list_item *items;
>  	unsigned int nr, alloc;
> -- 
> 2.14.1.821.g8fa685d3b7-goog
>

-- 
Brandon Williams