[4/5] argp: Improve comments.

  * argp/argp-help.c: Add sectioning comments. Write NULL to designate a
null pointer.
(struct hol_entry): Fix comment regarding sort order of group.
(hol_entry_short_iterate, hol_entry_long_iterate): Add comment.
(until_short, canon_doc_option, hol_entry_qcmp): Improve comment.
(hol_cluster_is_child, argp_hol): Move functions.
---
 argp/argp-help.c | 108 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 65 insertions(+), 43 deletions(-)
  

On 06/01/2021 22:06, Bruno Haible wrote:
> * argp/argp-help.c: Add sectioning comments. Write NULL to designate a
> null pointer.
> (struct hol_entry): Fix comment regarding sort order of group.
> (hol_entry_short_iterate, hol_entry_long_iterate): Add comment.
> (until_short, canon_doc_option, hol_entry_qcmp): Improve comment.
> (hol_cluster_is_child, argp_hol): Move functions.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  argp/argp-help.c | 108 +++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 65 insertions(+), 43 deletions(-)
> 
> diff --git a/argp/argp-help.c b/argp/argp-help.c
> index d686a23..637abca 100644
> --- a/argp/argp-help.c
> +++ b/argp/argp-help.c
> @@ -87,6 +87,8 @@ char *strerror (int errnum);
>  # define SIZE_MAX ((size_t) -1)
>  #endif
>  
> +/* ========================================================================== */
> +
>  /* User-selectable (using an environment variable) formatting parameters.
>  
>     These may be specified in an environment variable called `ARGP_HELP_FMT',
> @@ -253,6 +255,8 @@ fill_in_uparams (const struct argp_state *state)
>        }
>  }
>  
> +/* ========================================================================== */
> +
>  /* Returns true if OPT hasn't been marked invisible.  Visibility only affects
>     whether OPT is displayed or used in sorting, not option shadowing.  */
>  #define ovisible(opt) (! ((opt)->flags & OPTION_HIDDEN))
> @@ -345,6 +349,9 @@ find_char (char ch, char *beg, char *end)
>    return 0;
>  }
>  
> +/* -------------------------------------------------------------------------- */
> +/* Data structure: HOL = Help Option List                                     */
> +
>  struct hol_cluster;		/* fwd decl */
>  
>  struct hol_entry
> @@ -363,11 +370,11 @@ struct hol_entry
>    char *short_options;
>  
>    /* Entries are sorted by their group first, in the order:
> -       1, 2, ..., n, 0, -m, ..., -2, -1
> +       0, 1, 2, ..., n, -m, ..., -2, -1
>       and then alphabetically within each group.  The default is 0.  */
>    int group;
>  
> -  /* The cluster of options this entry belongs to, or 0 if none.  */
> +  /* The cluster of options this entry belongs to, or NULL if none.  */
>    struct hol_cluster *cluster;
>  
>    /* The argp from which this option came.  */
> @@ -389,7 +396,7 @@ struct hol_cluster
>       same depth (clusters always follow options in the same group).  */
>    int group;
>  
> -  /* The cluster to which this cluster belongs, or 0 if it's at the base
> +  /* The cluster to which this cluster belongs, or NULL if it's at the base
>       level.  */
>    struct hol_cluster *parent;
>  
> @@ -422,7 +429,7 @@ struct hol
>  };
>  
>  /* Create a struct hol from the options in ARGP.  CLUSTER is the
> -   hol_cluster in which these entries occur, or 0, if at the root.  */
> +   hol_cluster in which these entries occur, or NULL if at the root.  */
>  static struct hol *
>  make_hol (const struct argp *argp, struct hol_cluster *cluster)
>  {
> @@ -540,6 +547,9 @@ hol_free (struct hol *hol)
>    free (hol);
>  }
>  
> +/* Iterate across the short_options of the given ENTRY.  Call FUNC for each.
> +   Stop when such a call returns a non-zero value, and return this value.
> +   If all FUNC invocations returned 0, return 0.  */
>  static int
>  hol_entry_short_iterate (const struct hol_entry *entry,
>  			 int (*func)(const struct argp_option *opt,
> @@ -565,6 +575,9 @@ hol_entry_short_iterate (const struct hol_entry *entry,
>    return val;
>  }
>  
> +/* Iterate across the long options of the given ENTRY.  Call FUNC for each.
> +   Stop when such a call returns a non-zero value, and return this value.
> +   If all FUNC invocations returned 0, return 0.  */
>  static inline int
>  __attribute__ ((always_inline))
>  hol_entry_long_iterate (const struct hol_entry *entry,
> @@ -589,7 +602,7 @@ hol_entry_long_iterate (const struct hol_entry *entry,
>    return val;
>  }
>  
> -/* Iterator that returns true for the first short option.  */
> +/* A filter that returns true for the first short option of a given ENTRY.  */
>  static inline int
>  until_short (const struct argp_option *opt, const struct argp_option *real,
>  	     const char *domain, void *cookie)
> @@ -605,7 +618,7 @@ hol_entry_first_short (const struct hol_entry *entry)
>  				  entry->argp->argp_domain, 0);
>  }
>  
> -/* Returns the first valid long option in ENTRY, or 0 if there is none.  */
> +/* Returns the first valid long option in ENTRY, or NULL if there is none.  */
>  static const char *
>  hol_entry_first_long (const struct hol_entry *entry)
>  {
> @@ -617,7 +630,7 @@ hol_entry_first_long (const struct hol_entry *entry)
>    return 0;
>  }
>  
> -/* Returns the entry in HOL with the long option name NAME, or 0 if there is
> +/* Returns the entry in HOL with the long option name NAME, or NULL if there is
>     none.  */
>  static struct hol_entry *
>  hol_find_entry (struct hol *hol, const char *name)
> @@ -652,6 +665,9 @@ hol_set_group (struct hol *hol, const char *name, int group)
>      entry->group = group;
>  }
>  
> +/* -------------------------------------------------------------------------- */
> +/* Sorting the entries in a HOL.                                              */
> +
>  /* Order by group:  0, 1, 2, ..., n, -m, ..., -2, -1.
>     EQ is what to return if GROUP1 and GROUP2 are the same.  */
>  static int
> @@ -694,18 +710,8 @@ hol_cluster_base (struct hol_cluster *cl)
>      cl = cl->parent;
>    return cl;
>  }
> -
> -/* Return true if CL1 is a child of CL2.  */
> -static int
> -hol_cluster_is_child (const struct hol_cluster *cl1,
> -		      const struct hol_cluster *cl2)
> -{
> -  while (cl1 && cl1 != cl2)
> -    cl1 = cl1->parent;
> -  return cl1 == cl2;
> -}
>  
> -/* Given the name of a OPTION_DOC option, modifies NAME to start at the tail
> +/* Given the name of an OPTION_DOC option, modifies *NAME to start at the tail
>     that should be used for comparisons, and returns true iff it should be
>     treated as a non-option.  */
>  static int
> @@ -796,7 +802,7 @@ hol_entry_cmp (const struct hol_entry *entry1,
>      return group_cmp (group1, group2, 0);
>  }
>  
> -/* Version of hol_entry_cmp with correct signature for qsort.  */
> +/* Variant of hol_entry_cmp with correct signature for qsort.  */
>  static int
>  hol_entry_qcmp (const void *entry1_v, const void *entry2_v)
>  {
> @@ -814,6 +820,9 @@ hol_sort (struct hol *hol)
>  	   hol_entry_qcmp);
>  }
>  
> +/* -------------------------------------------------------------------------- */
> +/* Constructing the HOL.                                                      */
> +
>  /* Append MORE to HOL, destroying MORE in the process.  Options in HOL shadow
>     any in MORE with the same name.  */
>  static void
> @@ -909,6 +918,32 @@ hol_append (struct hol *hol, struct hol *more)
>    hol_free (more);
>  }
>  
> +/* Make a HOL containing all levels of options in ARGP.  CLUSTER is the
> +   cluster in which ARGP's entries should be clustered, or 0.  */
> +static struct hol *
> +argp_hol (const struct argp *argp, struct hol_cluster *cluster)
> +{
> +  const struct argp_child *child = argp->children;
> +  struct hol *hol = make_hol (argp, cluster);
> +  if (child)
> +    while (child->argp)
> +      {
> +	struct hol_cluster *child_cluster =
> +	  ((child->group || child->header)
> +	   /* Put CHILD->argp within its own cluster.  */
> +	   ? hol_add_cluster (hol, child->group, child->header,
> +			      child - argp->children, cluster, argp)
> +	   /* Just merge it into the parent's cluster.  */
> +	   : cluster);
> +	hol_append (hol, argp_hol (child->argp, child_cluster)) ;
> +	child++;
> +      }
> +  return hol;
> +}
> +
> +/* -------------------------------------------------------------------------- */
> +/* Printing the HOL.                                                          */
> +
>  /* Inserts enough spaces to make sure STREAM is at column COL.  */
>  static void
>  indent_to (argp_fmtstream_t stream, unsigned col)
> @@ -953,7 +988,7 @@ arg (const struct argp_option *real, const char *req_fmt, const char *opt_fmt,
>  /* State used during the execution of hol_help.  */
>  struct hol_help_state
>  {
> -  /* PREV_ENTRY should contain the previous entry printed, or 0.  */
> +  /* PREV_ENTRY should contain the previous entry printed, or NULL.  */
>    struct hol_entry *prev_entry;
>  
>    /* If an entry is in a different group from the previous one, and SEP_GROUPS
> @@ -1031,6 +1066,16 @@ print_header (const char *str, const struct argp *argp,
>      free ((char *) fstr);
>  }
>  
> +/* Return true if CL1 is a child of CL2.  */
> +static int
> +hol_cluster_is_child (const struct hol_cluster *cl1,
> +		      const struct hol_cluster *cl2)
> +{
> +  while (cl1 && cl1 != cl2)
> +    cl1 = cl1->parent;
> +  return cl1 == cl2;
> +}
> +
>  /* Inserts a comma if this isn't the first item on the line, and then makes
>     sure we're at least to column COL.  If this *is* the first item on a line,
>     prints any pending whitespace/headers that should precede this line. Also
> @@ -1344,29 +1389,6 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream)
>      }
>  }
>  
> -/* Make a HOL containing all levels of options in ARGP.  CLUSTER is the
> -   cluster in which ARGP's entries should be clustered, or 0.  */
> -static struct hol *
> -argp_hol (const struct argp *argp, struct hol_cluster *cluster)
> -{
> -  const struct argp_child *child = argp->children;
> -  struct hol *hol = make_hol (argp, cluster);
> -  if (child)
> -    while (child->argp)
> -      {
> -	struct hol_cluster *child_cluster =
> -	  ((child->group || child->header)
> -	   /* Put CHILD->argp within its own cluster.  */
> -	   ? hol_add_cluster (hol, child->group, child->header,
> -			      child - argp->children, cluster, argp)
> -	   /* Just merge it into the parent's cluster.  */
> -	   : cluster);
> -	hol_append (hol, argp_hol (child->argp, child_cluster)) ;
> -	child++;
> -      }
> -  return hol;
> -}
> -
>  /* Calculate how many different levels with alternative args strings exist in
>     ARGP.  */
>  static size_t
>