diff mbox series

[4/5] argp: Improve comments.

Message ID 1609981580-17229-5-git-send-email-bruno@clisp.org
State New
Headers show
Series argp: Fix several cases of undefined behaviour | expand

Commit Message

Bruno Haible Jan. 7, 2021, 1:06 a.m. UTC
* 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(-)
diff mbox series

Patch

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