[11/11] libctf, include: new functions for looking up enumerators

  Three new functions for looking up the enum type containing a given
enumeration constant, and optionally that constant's value.

The simplest, ctf_lookup_enumerator, looks up a root-visible enumerator by
name in one dict: if the dict contains multiple such constants (which is
possible for dicts created by older versions of hte libctf deduplicator),
ECTF_DUPLICATE is returned.

The next simplest, ctf_lookup_enumerator_next, is an iterator which returns
all enumerators with ag iven name in a given dict, whether root-visible or
not.

The most elaborate, ctf_arc_lookup_enumerator_next, finds all
enumerators with a given name across all dicts in an entire CTF archive,
whether root-visible or not, starting looking in the shared parent dict;
opened dicts are cached (as with all other ctf_arc_*lookup functions) so
that repeated use does not incur repeated opening costs.

All three of these return enumerator values as int64_t: unfortunately, API
compatibilty concerns prevent us from doing the same with the other older
enum-related functions, which all return enumerator constant values as ints.
We may be forced to add symbol-versioning compatibility aliases that fix the
other functions in due course, bumping the soname for platforms that do not
support such things.

ctf_arc_lookup_enumerator_next is implemented as a nested ctf_archive_next
iterator, and inside that, a nested ctf_lookup_enumerator_next iterator
within each dict.  To aid in this, add support to ctf_next_t iterators for
iterators that are implemented in terms of two simultaneous nested iterators
at once.  (It has always been possible for callers to use as many nested or
semi-overlapping ctf_next_t iterators as they need, which is one of the
advantages of this style over the _iter style that calls a function for each
thing iterated over: the iterator change here permits *ctf_next_t iterators
themselves* to be implemented by iterating using multiple other iterators as
part of their internal operation, transparently to the caller.)

Also add a testcase that tests all these functions (which is fairly easy
because ctf_arc_lookup_enumerator_next is implemented in terms of
ctf_lookup_enumerator_next) in addition to enumeration addition in
ctf_open()ed dicts, ctf_add_enumerator duplicate enumerator addition, and
conflicting enumerator constant deduplication.

include/
	* ctf-api.h (ctf_lookup_enumerator): New.
	(ctf_lookup_enumerator_next): Likewise.
	(ctf_arc_lookup_enumerator_next): Likewise.

libctf/
	* libctf.ver: Add them.
	* ctf-impl.h (ctf_next_t) <ctn_next_inner>: New.
	* ctf-util.c (ctf_next_copy): Copy it.
        (ctf_next_destroy): Destroy it.
	* ctf-lookup.c (ctf_lookup_enumerator): New.
	(ctf_lookup_enumerator_next): New.
	* ctf-archive.c (ctf_arc_lookup_enumerator_next): New.
	* testsuite/libctf-lookup/enumerator-iteration.*: New test.
	* testsuite/libctf-lookup/enum-ctf-2.c: New test CTF, used by the
	  above.
---
 include/ctf-api.h                             |  37 ++++
 libctf/ctf-archive.c                          | 107 +++++++++++
 libctf/ctf-impl.h                             |  12 +-
 libctf/ctf-lookup.c                           | 145 +++++++++++++++
 libctf/ctf-util.c                             |  29 ++-
 libctf/libctf.ver                             |   7 +
 libctf/testsuite/libctf-lookup/enum-ctf-2.c   |   6 +
 .../libctf-lookup/enumerator-iteration.c      | 168 ++++++++++++++++++
 .../libctf-lookup/enumerator-iteration.lk     |  17 ++
 9 files changed, 519 insertions(+), 9 deletions(-)
 create mode 100644 libctf/testsuite/libctf-lookup/enum-ctf-2.c
 create mode 100644 libctf/testsuite/libctf-lookup/enumerator-iteration.c
 create mode 100644 libctf/testsuite/libctf-lookup/enumerator-iteration.lk
  

On 6/13/24 11:54, Nick Alcock wrote:
> Three new functions for looking up the enum type containing a given
> enumeration constant, and optionally that constant's value.
> 

The new functions make sense to me, though I don't have much experience
as a user of libctf or it's APIs.

Just a few small comments/nits below.

> The simplest, ctf_lookup_enumerator, looks up a root-visible enumerator by
> name in one dict: if the dict contains multiple such constants (which is
> possible for dicts created by older versions of hte libctf deduplicator),
> ECTF_DUPLICATE is returned.

typo: hte

> 
> The next simplest, ctf_lookup_enumerator_next, is an iterator which returns
> all enumerators with ag iven name in a given dict, whether root-visible or
> not.

typo: ag iven

> 
> The most elaborate, ctf_arc_lookup_enumerator_next, finds all
> enumerators with a given name across all dicts in an entire CTF archive,
> whether root-visible or not, starting looking in the shared parent dict;
> opened dicts are cached (as with all other ctf_arc_*lookup functions) so
> that repeated use does not incur repeated opening costs.
> 
> All three of these return enumerator values as int64_t: unfortunately, API
> compatibilty concerns prevent us from doing the same with the other older

typo: compatibilty

> enum-related functions, which all return enumerator constant values as ints.
> We may be forced to add symbol-versioning compatibility aliases that fix the
> other functions in due course, bumping the soname for platforms that do not
> support such things.
> 
> ctf_arc_lookup_enumerator_next is implemented as a nested ctf_archive_next
> iterator, and inside that, a nested ctf_lookup_enumerator_next iterator
> within each dict.  To aid in this, add support to ctf_next_t iterators for
> iterators that are implemented in terms of two simultaneous nested iterators
> at once.  (It has always been possible for callers to use as many nested or
> semi-overlapping ctf_next_t iterators as they need, which is one of the
> advantages of this style over the _iter style that calls a function for each
> thing iterated over: the iterator change here permits *ctf_next_t iterators
> themselves* to be implemented by iterating using multiple other iterators as
> part of their internal operation, transparently to the caller.)
> 
> Also add a testcase that tests all these functions (which is fairly easy
> because ctf_arc_lookup_enumerator_next is implemented in terms of
> ctf_lookup_enumerator_next) in addition to enumeration addition in
> ctf_open()ed dicts, ctf_add_enumerator duplicate enumerator addition, and
> conflicting enumerator constant deduplication.
> 
> include/
> 	* ctf-api.h (ctf_lookup_enumerator): New.
> 	(ctf_lookup_enumerator_next): Likewise.
> 	(ctf_arc_lookup_enumerator_next): Likewise.
> 
> libctf/
> 	* libctf.ver: Add them.
> 	* ctf-impl.h (ctf_next_t) <ctn_next_inner>: New.
> 	* ctf-util.c (ctf_next_copy): Copy it.
>         (ctf_next_destroy): Destroy it.
> 	* ctf-lookup.c (ctf_lookup_enumerator): New.
> 	(ctf_lookup_enumerator_next): New.
> 	* ctf-archive.c (ctf_arc_lookup_enumerator_next): New.
> 	* testsuite/libctf-lookup/enumerator-iteration.*: New test.
> 	* testsuite/libctf-lookup/enum-ctf-2.c: New test CTF, used by the
> 	  above.
> ---
>  include/ctf-api.h                             |  37 ++++
>  libctf/ctf-archive.c                          | 107 +++++++++++
>  libctf/ctf-impl.h                             |  12 +-
>  libctf/ctf-lookup.c                           | 145 +++++++++++++++
>  libctf/ctf-util.c                             |  29 ++-
>  libctf/libctf.ver                             |   7 +
>  libctf/testsuite/libctf-lookup/enum-ctf-2.c   |   6 +
>  .../libctf-lookup/enumerator-iteration.c      | 168 ++++++++++++++++++
>  .../libctf-lookup/enumerator-iteration.lk     |  17 ++
>  9 files changed, 519 insertions(+), 9 deletions(-)
>  create mode 100644 libctf/testsuite/libctf-lookup/enum-ctf-2.c
>  create mode 100644 libctf/testsuite/libctf-lookup/enumerator-iteration.c
>  create mode 100644 libctf/testsuite/libctf-lookup/enumerator-iteration.lk
> 
> diff --git a/include/ctf-api.h b/include/ctf-api.h
> index d67db8be13f..d3bd1b586d8 100644
> --- a/include/ctf-api.h
> +++ b/include/ctf-api.h
> @@ -25,6 +25,7 @@
>  #define	_CTF_API_H
>  
>  #include <sys/types.h>
> +#include <inttypes.h>
>  #include <ctf.h>
>  #include <zlib.h>
>  
> @@ -538,6 +539,16 @@ extern ctf_id_t ctf_lookup_by_name (ctf_dict_t *, const char *);
>  
>  extern ctf_id_t ctf_lookup_variable (ctf_dict_t *, const char *);
>  
> +/* Look up a single enumerator by enumeration constant name.  Returns the ID of
> +   the enum it is contained within and optionally its value.  Error out with
> +   ECTF_DUPLICATE if multiple exist (which can happen in some older dicts).  See
> +   ctf_lookup_enumerator_next in that case.  Enumeration constants in non-root
> +   types are not returned, but constants in parents are, if not overridden by
> +   an enum in the child.  */
> +
> +extern ctf_id_t ctf_lookup_enumerator (ctf_dict_t *, const char *,
> +				       int64_t *enum_value);
> +
>  /* Type lookup functions.  */
>  
>  /* Strip qualifiers and typedefs off a type, returning the base type.
> @@ -669,6 +680,32 @@ extern int ctf_enum_iter (ctf_dict_t *, ctf_id_t, ctf_enum_f *, void *);
>  extern const char *ctf_enum_next (ctf_dict_t *, ctf_id_t, ctf_next_t **,
>  				  int *);
>  
> +/* Return all enumeration constants with a given name in a given dict, similar
> +   to ctf_lookup_enumerator above but capable of returning multiple values.
> +   Enumerators in parent dictionaries are not returned: enumerators in non-root
> +   types *are* returned.  This operation internally iterates over all types in
> +   the dict, so is relatively expensive in large dictionaries.
> +
> +   There is nothing preventing NAME from being changed by the caller in the
> +   middle of iteration: the results might be slightly confusing, but they are
> +   well-defined.  */
> +
> +extern ctf_id_t ctf_lookup_enumerator_next (ctf_dict_t *, const char *name,
> +					    ctf_next_t **, int64_t *enum_value);
> +
> +/* Likewise, across all dicts in an archive (parent first).  The DICT and ERRP
> +   arguments are not optional: without the one you can't tell which dict the
> +   returned type is in, and without the other you can't distinguish real errors

Maybe "without the former... and without the latter..." would be
more clear?

> +   from end-of-iteration.  DICT should be NULL before the first call and is set
> +   to NULL after the last and on error: on successful call it is the caller's
> +   responsibility to ctf_dict_close() it.  The caller should  otherwise pass it

IMO it would be good to also clearly say that on success DICT is set to
point to the dictionary containing the returned type.  (Which has been
implicitly opened and therefore must be closed via ctf_dict_close by the
caller).


> +   back in unchanged (do not reassign it during iteration, just as with the
> +   ctf_next_t iterator itself).  */
> +
> +extern ctf_id_t ctf_arc_lookup_enumerator_next (ctf_archive_t *, const char *name,
> +						ctf_next_t **, int64_t *enum_value,
> +						ctf_dict_t **dict, int *errp);
> +
>  /* Iterate over all types in a dict.  ctf_type_iter_all recurses over all types:
>     ctf_type_iter recurses only over types with user-visible names (for which
>     CTF_ADD_ROOT was passed).  All such types are returned, even if they are
> diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
> index 4744cb7a828..3ba6a3efabb 100644
> --- a/libctf/ctf-archive.c
> +++ b/libctf/ctf-archive.c
> @@ -1011,6 +1011,113 @@ ctf_arc_lookup_symbol_name (ctf_archive_t *wrapper, const char *symname,
>    return ctf_arc_lookup_sym_or_name (wrapper, 0, symname, typep, errp);
>  }
>  
> +/* Return all enumeration constants with a given name across all dicts in an
> +   archive, similar to ctf_lookup_enumerator_next.  The dict is cached, so
> +   opening costs are paid only once, but (unlike ctf_arc_lookup_symbol*
> +   above) the results of the iterations are not cached.  dict and errp are
> +   not optional.  */

Little inconsistency in dict and errp not being capitalized like above.

> +
> +ctf_id_t
> +ctf_arc_lookup_enumerator_next (ctf_archive_t *arc, const char *name,
> +				ctf_next_t **it, int64_t *enum_value,
> +				ctf_dict_t **dict, int *errp)
> +{
> +  ctf_next_t *i = *it;
> +  ctf_id_t type;
> +  int opened_this_time = 0;
> +  int err;
> +
> +  /* We have two nested iterators in here: ctn_next tracks archives, while
> +     within it ctn_next_inner tracks enumerators within an archive.  We
> +     keep track of the dict by simply reusing the passed-in arg: if it's
> +     changed by the caller, the caller will get an ECTF_WRONGFP error,
> +     so this is quite safe and means we don't have to track the arc and fp
> +     simultaneously in the ctf_next_t.  */
> +
> +  if (!i)
> +    {
> +      if ((i = ctf_next_create ()) == NULL)
> +	{
> +	  err = ENOMEM;
> +	  goto err;
> +	}
> +      i->ctn_iter_fun = (void (*) (void)) ctf_arc_lookup_enumerator_next;
> +      i->cu.ctn_arc = arc;
> +      *it = i;
> +    }
> +
> +  if ((void (*) (void)) ctf_arc_lookup_enumerator_next != i->ctn_iter_fun)
> +    {
> +      err = ECTF_NEXT_WRONGFUN;
> +      goto err;
> +    }
> +
> +  if (arc != i->cu.ctn_arc)
> +    {
> +      err = ECTF_NEXT_WRONGFP;
> +      goto err;
> +    }
> +
> +  /* Prevent any earlier end-of-iteration on this dict from confusing the
> +     test below.  */
> +  if (i->ctn_next != NULL)
> +    ctf_set_errno (*dict, 0);
> +
> +  do
> +    {
> +      /* At end of one dict, or not started any iterations yet?
> +	 Traverse to next dict.  If we never returned this dict to the
> +	 caller, close it ourselves: the caller will never see it and cannot
> +	 do so.  */
> +
> +      if (i->ctn_next == NULL || ctf_errno (*dict) == ECTF_NEXT_END)
> +	{
> +	  if (opened_this_time)
> +	    {
> +	      ctf_dict_close (*dict);
> +	      *dict = NULL;
> +	      opened_this_time = 0;
> +	    }
> +
> +	  *dict = ctf_archive_next (arc, &i->ctn_next, NULL, 0, &err);
> +	  if (!*dict)
> +	    goto err;
> +	  opened_this_time = 1;
> +	}
> +
> +      type = ctf_lookup_enumerator_next (*dict, name, &i->ctn_next_inner,
> +					 enum_value);
> +    }
> +  while (type == CTF_ERR && ctf_errno (*dict) == ECTF_NEXT_END);
> +
> +  if (type == CTF_ERR)
> +    {
> +      err = ctf_errno (*dict);
> +      goto err;
> +    }
> +
> +  /* If this dict is being reused from the previous iteration, bump its
> +     refcnt: the caller is going to close it and has no idea that we didn't
> +     open it this time round.  */
> +  if (!opened_this_time)
> +    ctf_ref (*dict);
> +
> +  return type;
> +
> + err:						/* Also ECTF_NEXT_END. */
> +  if (opened_this_time)
> +    {
> +      ctf_dict_close (*dict);
> +      *dict = NULL;
> +    }
> +
> +  ctf_next_destroy (i);
> +  *it = NULL;
> +  if (errp)
> +    *errp = err;
> +  return CTF_ERR;
> +}
> +
>  /* Raw iteration over all CTF files in an archive.  We pass the raw data for all
>     CTF files in turn to the specified callback function.  */
>  static int
> diff --git a/libctf/ctf-impl.h b/libctf/ctf-impl.h
> index 299d981a718..0a362b6b17c 100644
> --- a/libctf/ctf-impl.h
> +++ b/libctf/ctf-impl.h
> @@ -544,13 +544,15 @@ struct ctf_next
>    uint32_t ctn_n;
>  
>    /* Some iterators contain other iterators, in addition to their other
> -     state.  */
> +     state.  We allow for inner and outer iterators, for two-layer nested loops
> +     like those found in ctf_arc_lookup_enumerator_next.  */
>    ctf_next_t *ctn_next;
> +  ctf_next_t *ctn_next_inner;
>  
> -  /* We can save space on this side of things by noting that a dictionary is
> -     either dynamic or not, as a whole, and a given iterator can only iterate
> -     over one kind of thing at once: so we can overlap the DTD and non-DTD
> -     members, and the structure, variable and enum members, etc.  */
> +  /* We can save space on this side of things by noting that a type is either
> +     dynamic or not, as a whole, and a given iterator can only iterate over one
> +     kind of thing at once: so we can overlap the DTD and non-DTD members, and
> +     the structure, variable and enum members, etc.  */
>    union
>    {
>      unsigned char *ctn_vlen;
> diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
> index e4d18bec112..8accb2ed99e 100644
> --- a/libctf/ctf-lookup.c
> +++ b/libctf/ctf-lookup.c
> @@ -413,6 +413,151 @@ ctf_lookup_variable (ctf_dict_t *fp, const char *name)
>    return type;
>  }
>  
> +/* Look up a single enumerator by enumeration constant name.  Returns the ID of
> +   the enum it is contained within and optionally its value.  Error out with
> +   ECTF_DUPLICATE if multiple exist (which can happen in some older dicts).  See
> +   ctf_lookup_enumerator_next in that case.  Enumeration constants in non-root
> +   types are not returned, but constants in parents are, if not overridden by
> +   an enum in the child..  */
> +
> +ctf_id_t
> +ctf_lookup_enumerator (ctf_dict_t *fp, const char *name, int64_t *enum_value)
> +{
> +  ctf_id_t type;
> +  int enum_int_value;
> +
> +  if (ctf_dynset_lookup (fp->ctf_conflicting_enums, name))
> +    return (ctf_set_typed_errno (fp, ECTF_DUPLICATE));
> +
> +  /* CTF_K_UNKNOWN suffices for things like enumeration constants that aren't
> +     actually types at all (ending up in the global name table).  */
> +  type = ctf_lookup_by_rawname (fp, CTF_K_UNKNOWN, name);
> +  /* Nonexistent type? It may be in the parent.  */
> +  if (type == 0 && fp->ctf_parent)
> +    {
> +      if ((type = ctf_lookup_enumerator (fp->ctf_parent, name, enum_value)) == 0)
> +	return ctf_set_typed_errno (fp, ECTF_NOENUMNAM);
> +      return type;
> +    }
> +
> +  /* Nothing more to do if this type didn't exist or we don't have to look up
> +     the enum value.  */
> +  if (type == 0)
> +    return ctf_set_typed_errno (fp, ECTF_NOENUMNAM);
> +
> +  if (enum_value == NULL)
> +    return type;
> +
> +  if (ctf_enum_value (fp, type, name, &enum_int_value) < 0)
> +    return CTF_ERR;
> +  *enum_value = enum_int_value;
> +
> +  return type;
> +}
> +
> +/* Return all enumeration constants with a given name in a given dict, similar
> +   to ctf_lookup_enumerator above but capable of returning multiple values.
> +   Enumerators in parent dictionaries are not returned: enumerators in
> +   hidden types *are* returned.  */
> +
> +ctf_id_t
> +ctf_lookup_enumerator_next (ctf_dict_t *fp, const char *name,
> +			    ctf_next_t **it, int64_t *val)
> +{
> +  ctf_next_t *i = *it;
> +  int found = 0;
> +
> +  /* We use ctf_type_next() to iterate across all types, but then traverse each
> +     enumerator found by hand: traversing enumerators is very easy, and it would
> +     probably be more confusing to use two nested iterators than to do it this
> +     way.  We use ctn_next to work over enums, then ctn_en and ctn_n to work
> +     over enumerators within each enum.  */
> +  if (!i)
> +    {
> +      if ((i = ctf_next_create ()) == NULL)
> +	return ctf_set_typed_errno (fp, ENOMEM);
> +
> +      i->cu.ctn_fp = fp;
> +      i->ctn_iter_fun = (void (*) (void)) ctf_lookup_enumerator_next;
> +      i->ctn_increment = 0;
> +      i->ctn_tp = NULL;
> +      i->u.ctn_en = NULL;
> +      i->ctn_n = 0;
> +      *it = i;
> +    }
> +
> +  if ((void (*) (void)) ctf_lookup_enumerator_next != i->ctn_iter_fun)
> +    return (ctf_set_typed_errno (fp, ECTF_NEXT_WRONGFUN));
> +
> +  if (fp != i->cu.ctn_fp)
> +    return (ctf_set_typed_errno (fp, ECTF_NEXT_WRONGFP));
> +
> +  do
> +    {
> +      const char *this_name;
> +
> +      /* At end of enum? Traverse to next one, if any are left.  */
> +
> +      if (i->u.ctn_en == NULL || i->ctn_n == 0)
> +	{
> +	  const ctf_type_t *tp;
> +	  ctf_dtdef_t *dtd;
> +
> +	  do
> +	    i->ctn_type = ctf_type_next (i->cu.ctn_fp, &i->ctn_next, NULL, 1);
> +	  while (i->ctn_type != CTF_ERR
> +		 && ctf_type_kind_unsliced (i->cu.ctn_fp, i->ctn_type)
> +		 != CTF_K_ENUM);
> +
> +	  if (i->ctn_type == CTF_ERR)
> +	    {
> +	      /* Conveniently, when the iterator over all types is done, so is the
> +		 iteration as a whole: so we can just pass all errors from the
> +		 internal iterator straight back out..  */
> +	      ctf_next_destroy (i);
> +	      *it = NULL;
> +	      return CTF_ERR;			/* errno is set for us.  */
> +	    }
> +
> +	  if ((tp = ctf_lookup_by_id (&fp, i->ctn_type)) == NULL)
> +	    return CTF_ERR;			/* errno is set for us.  */
> +	  i->ctn_n = LCTF_INFO_VLEN (fp, tp->ctt_info);
> +
> +	  dtd = ctf_dynamic_type (fp, i->ctn_type);
> +
> +	  if (dtd == NULL)
> +	    {
> +	      (void) ctf_get_ctt_size (fp, tp, NULL, &i->ctn_increment);
> +	      i->u.ctn_en = (const ctf_enum_t *) ((uintptr_t) tp +
> +						  i->ctn_increment);
> +	    }
> +	  else
> +	    i->u.ctn_en = (const ctf_enum_t *) dtd->dtd_vlen;
> +	}
> +
> +      this_name = ctf_strptr (fp, i->u.ctn_en->cte_name);
> +
> +      i->ctn_n--;
> +
> +      if (strcmp (name, this_name) == 0)
> +	{
> +	  if (val)
> +	    *val = i->u.ctn_en->cte_value;
> +	  found = 1;
> +
> +	  /* Constant found in this enum: try the next one.  (Constant names
> +	     cannot be duplicated within a given enum.)  */
> +
> +	  i->ctn_n = 0;
> +	}
> +
> +      i->u.ctn_en++;
> +    }
> +  while (!found);
> +
> +  return i->ctn_type;
> +}
> +
>  typedef struct ctf_symidx_sort_arg_cb
>  {
>    ctf_dict_t *fp;
> diff --git a/libctf/ctf-util.c b/libctf/ctf-util.c
> index 3ea6de9e86f..f75b1bfb01a 100644
> --- a/libctf/ctf-util.c
> +++ b/libctf/ctf-util.c
> @@ -262,6 +262,8 @@ ctf_next_destroy (ctf_next_t *i)
>      free (i->u.ctn_sorted_hkv);
>    if (i->ctn_next)
>      ctf_next_destroy (i->ctn_next);
> +  if (i->ctn_next_inner)
> +    ctf_next_destroy (i->ctn_next_inner);
>    free (i);
>  }
>  
> @@ -276,16 +278,35 @@ ctf_next_copy (ctf_next_t *i)
>      return NULL;
>    memcpy (i2, i, sizeof (struct ctf_next));
>  
> +  if (i2->ctn_next)
> +    {
> +      i2->ctn_next = ctf_next_copy (i2->ctn_next);
> +      if (i2->ctn_next == NULL)
> +	goto err_next;
> +    }
> +
> +  if (i2->ctn_next_inner)
> +    {
> +      i2->ctn_next_inner = ctf_next_copy (i2->ctn_next_inner);
> +      if (i2->ctn_next_inner == NULL)
> +	goto err_next_inner;
> +    }
> +
>    if (i2->ctn_iter_fun == (void (*) (void)) ctf_dynhash_next_sorted)
>      {
>        size_t els = ctf_dynhash_elements ((ctf_dynhash_t *) i->cu.ctn_h);
>        if ((i2->u.ctn_sorted_hkv = calloc (els, sizeof (ctf_next_hkv_t))) == NULL)
> -	{
> -	  free (i2);
> -	  return NULL;
> -	}
> +	goto err_sorted_hkv;
>        memcpy (i2->u.ctn_sorted_hkv, i->u.ctn_sorted_hkv,
>  	      els * sizeof (ctf_next_hkv_t));
>      }
>    return i2;
> +
> + err_sorted_hkv:
> +  ctf_next_destroy (i2->ctn_next_inner);
> + err_next_inner:
> +  ctf_next_destroy (i2->ctn_next);
> + err_next:
> +  ctf_next_destroy (i2);
> +  return NULL;
>  }
> diff --git a/libctf/libctf.ver b/libctf/libctf.ver
> index 6e7345be66b..e6c31ff37aa 100644
> --- a/libctf/libctf.ver
> +++ b/libctf/libctf.ver
> @@ -198,3 +198,10 @@ LIBCTF_1.2 {
>  	ctf_arc_lookup_symbol_name;
>  	ctf_add_unknown;
>  } LIBCTF_1.1;
> +
> +LIBCTF_1.3 {
> +    global:
> +	ctf_lookup_enumerator;
> +	ctf_lookup_enumerator_next;
> +	ctf_arc_lookup_enumerator_next;
> +} LIBCTF_1.2;
> diff --git a/libctf/testsuite/libctf-lookup/enum-ctf-2.c b/libctf/testsuite/libctf-lookup/enum-ctf-2.c
> new file mode 100644
> index 00000000000..39c9865e528
> --- /dev/null
> +++ b/libctf/testsuite/libctf-lookup/enum-ctf-2.c
> @@ -0,0 +1,6 @@
> +enum e { ENUMSAMPLE_1 = 6, ENUMSAMPLE_2 = 7 };
> +
> +enum ie2 { IENUMSAMPLE2_1 = -10, IENUMSAMPLE2_2 };
> +
> +enum e baz;
> +enum ie2 quux;
> diff --git a/libctf/testsuite/libctf-lookup/enumerator-iteration.c b/libctf/testsuite/libctf-lookup/enumerator-iteration.c
> new file mode 100644
> index 00000000000..e46dad6dc70
> --- /dev/null
> +++ b/libctf/testsuite/libctf-lookup/enumerator-iteration.c
> @@ -0,0 +1,168 @@
> +/* Test enumerator iteration and querying.  Because
> +   ctf_arc_lookup_enumerator_next uses ctf_lookup_enumerator_next internally, we
> +   only need to test the former.  */
> +
> +#include "config.h"
> +#include <ctf-api.h>
> +#include <inttypes.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +static void
> +print_constants (ctf_archive_t *ctf, const char *name)
> +{
> +  ctf_next_t *i = NULL;
> +  int err;
> +  ctf_dict_t *fp;
> +  ctf_id_t type;
> +  int64_t val;
> +
> +  while ((type = ctf_arc_lookup_enumerator_next (ctf, name, &i,
> +						 &val, &fp, &err)) != CTF_ERR)
> +    {
> +      char *foo;
> +
> +      printf ("%s in %s has value %i\n", name,
> +	      foo = ctf_type_aname (fp, type), val);
> +      free (foo);
> +
> +      ctf_dict_close (fp);
> +    }
> +  if (err != ECTF_NEXT_END)
> +    {
> +      fprintf (stderr, "iteration failed: %s\n", ctf_errmsg (err));
> +      exit (1);
> +    }
> +}
> +
> +int
> +main (int argc, char *argv[])
> +{
> +  ctf_archive_t *ctf;
> +  ctf_dict_t *fp;
> +  int err;
> +  ctf_id_t type;
> +  ctf_next_t *i = NULL;
> +  const char *name;
> +  int64_t val;
> +  int counter = 0;
> +
> +  if (argc != 2)
> +    {
> +      fprintf (stderr, "Syntax: %s PROGRAM\n", argv[0]);
> +      exit(1);
> +    }
> +
> +  if ((ctf = ctf_open (argv[1], NULL, &err)) == NULL)
> +    goto open_err;
> +
> +  /* Look for all instances of ENUMSAMPLE2_1, and add some new enums to all
> +     dicts found, to test dynamic enum iteration as well as static.
> +
> +     Add two enums with a different name and constants to any that should
> +     already be there (one hidden), and one with the same constants, but hidden,
> +     to test ctf_lookup_enumerator_next()'s multiple-lookup functionality and
> +     ctf_lookup_enumerator() in the presence of hidden types.  */
> +
> +  printf ("First iteration: addition of enums.\n");
> +  while ((type = ctf_arc_lookup_enumerator_next (ctf, "IENUMSAMPLE2_2", &i,
> +						 &val, &fp, &err)) != CTF_ERR)
> +    {
> +      char *foo;
> +
> +      printf ("IENUMSAMPLE2_2 in %s has value %i\n",
> +	      foo = ctf_type_aname (fp, type), val);
> +      free (foo);
> +
> +      if ((type = ctf_add_enum (fp, CTF_ADD_ROOT, "ie3")) == CTF_ERR)
> +	goto enum_add_err;
> +
> +      if (ctf_add_enumerator (fp, type, "DYNADD", counter += 10) < 0)
> +	goto enumerator_add_err;
> +      if (ctf_add_enumerator (fp, type, "DYNADD2", counter += 10) < 0)
> +	goto enumerator_add_err;
> +
> +      /* Make sure that overlapping enumerator addition fails as it should.  */
> +
> +      if (ctf_add_enumerator (fp, type, "IENUMSAMPLE2_2", 666) >= 0
> +	  || ctf_errno (fp) != ECTF_DUPLICATE)
> +	fprintf (stderr, "Duplicate enumerator addition did not fail as it ought to\n");
> +
> +      if ((type = ctf_add_enum (fp, CTF_ADD_NONROOT, "ie4_hidden")) == CTF_ERR)
> +	goto enum_add_err;
> +
> +      if (ctf_add_enumerator (fp, type, "DYNADD3", counter += 10) < 0)
> +	goto enumerator_add_err;
> +      if (ctf_add_enumerator (fp, type, "DYNADD4", counter += 10) < 0)
> +	goto enumerator_add_err;
> +
> +      if ((type = ctf_add_enum (fp, CTF_ADD_NONROOT, "ie3_hidden")) == CTF_ERR)
> +	goto enum_add_err;
> +
> +      if (ctf_add_enumerator (fp, type, "DYNADD", counter += 10) < 0)
> +	goto enumerator_add_err;
> +      if (ctf_add_enumerator (fp, type, "DYNADD2", counter += 10) < 0)
> +	goto enumerator_add_err;
> +
> +      /* Look them up via ctf_lookup_enumerator.  */
> +
> +      if (ctf_lookup_enumerator (fp, "DYNADD", &val) == CTF_ERR)
> +	goto enumerator_lookup_err;
> +      printf ("direct lookup: DYNADD value: %i\n", (int) val);
> +
> +      if ((type = ctf_lookup_enumerator (fp, "DYNADD3", &val) != CTF_ERR) ||
> +	  ctf_errno (fp) != ECTF_NOENUMNAM)
> +	{
> +	  if (type != CTF_ERR)
> +	    {
> +	      char *foo;
> +	      printf ("direct lookup: hidden lookup did not return ECTF_NOENUMNAM but rather %i in %s\n",
> +		      val, foo = ctf_type_aname (fp, type));
> +	      free (foo);
> +	    }
> +	  else
> +	    printf ("direct lookup: hidden lookup did not return ECTF_NOENUMNAM but rather %s\n",
> +		    ctf_errno (fp));
> +	}
> +
> +      ctf_dict_close (fp);
> +    }
> +  if (err != ECTF_NEXT_END)
> +    {
> +      fprintf (stderr, "iteration failed: %s\n", ctf_errmsg (err));
> +      return 1;
> +    }
> +
> +  /* Look for (and print out) some enumeration constants.  */
> +
> +  printf ("Second iteration: printing of enums.\n");
> +
> +  print_constants (ctf, "ENUMSAMPLE_1");
> +  print_constants (ctf, "IENUMSAMPLE_1");
> +  print_constants (ctf, "ENUMSAMPLE_2");
> +  print_constants (ctf, "DYNADD");
> +  print_constants (ctf, "DYNADD3");
> +
> +  ctf_close (ctf);
> +
> +  printf ("All done.\n");
> +
> +  return 0;
> +
> + open_err:
> +  fprintf (stderr, "%s: cannot open: %s\n", argv[0], ctf_errmsg (err));
> +  return 1;
> + enum_add_err:
> +  fprintf (stderr, "Cannot add enum to dict \"%s\": %s\n",
> +	   ctf_cuname (fp) ? ctf_cuname (fp) : "(null: parent)", ctf_errmsg (ctf_errno (fp)));
> +  return 1;
> + enumerator_add_err:
> +  fprintf (stderr, "Cannot add enumerator to dict \"%s\": %s\n",
> +	   ctf_cuname (fp) ? ctf_cuname (fp) : "(null: parent)", ctf_errmsg (ctf_errno (fp)));
> +  return 1;
> + enumerator_lookup_err:
> +  fprintf (stderr, "Cannot look up enumerator in dict \"%s\": %s\n",
> +	   ctf_cuname (fp) ? ctf_cuname (fp) : "(null: parent)", ctf_errmsg (ctf_errno (fp)));
> +  return 1;
> +}
> diff --git a/libctf/testsuite/libctf-lookup/enumerator-iteration.lk b/libctf/testsuite/libctf-lookup/enumerator-iteration.lk
> new file mode 100644
> index 00000000000..0c3cbfbf15f
> --- /dev/null
> +++ b/libctf/testsuite/libctf-lookup/enumerator-iteration.lk
> @@ -0,0 +1,17 @@
> +# lookup: enumerator-iteration.c
> +# source: enum-ctf.c
> +# source: enum-ctf-2.c
> +# link: on
> +First iteration: addition of enums.
> +IENUMSAMPLE2_2 in enum ie2 has value -9
> +direct lookup: DYNADD value: 10
> +Second iteration: printing of enums.
> +ENUMSAMPLE_1 in enum e has value 6
> +ENUMSAMPLE_1 in enum e has value 0
> +IENUMSAMPLE_1 in enum ie has value -10
> +ENUMSAMPLE_2 in enum e has value 7
> +ENUMSAMPLE_2 in enum e has value 1
> +DYNADD in enum ie3 has value 10
> +DYNADD in enum ie3_hidden has value 50
> +DYNADD3 in enum ie4_hidden has value 30
> +All done.
  

On 14 Jun 2024, David Faust uttered the following:

> On 6/13/24 11:54, Nick Alcock wrote:
>> Three new functions for looking up the enum type containing a given
>> enumeration constant, and optionally that constant's value.
>
> The new functions make sense to me, though I don't have much experience
> as a user of libctf or it's APIs.

The APIs I got help on from Stephen Brennan and Indu Bhagat earlier. I'm
fairly confident that at least *one* user will like them. :)

> Just a few small comments/nits below.

Thanks! All fixed as you suggested.

>> +   from end-of-iteration.  DICT should be NULL before the first call and is set
>> +   to NULL after the last and on error: on successful call it is the caller's
>> +   responsibility to ctf_dict_close() it.  The caller should  otherwise pass it
>
> IMO it would be good to also clearly say that on success DICT is set to
> point to the dictionary containing the returned type.  (Which has been
> implicitly opened and therefore must be closed via ctf_dict_close by the
> caller).

True! I, uh, assumed that would go without saying, which in
documentation is probably a bad stance :)

The "pass in DICT unchanged, we actually use it to store state" thing is
a bit weird but seems more or less harmless (when I get dict leaks, it's
not because of that, it's because I forget to close them on error
paths), and without it I'd need to add the ability for ctf_next_t
iterators to hold archives and dicts at the same time, which does get
quite complex. (I tried, and honestly the _next iterators are complex
enough to write as it is!)