[FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate

[Andreas Rheinhardt]

Marvin Scholz:
> This is a more explicit iteration API rather than using the "magic"
> av_dict_get(d, "", t, AV_DICT_IGNORE_SUFFIX) which is not really
> trivial to grasp what it does when casually reading through code.
> ---
>  libavutil/dict.c    | 19 +++++++++++++++++++
>  libavutil/dict.h    | 27 +++++++++++++++++++++++++++
>  libavutil/version.h |  2 +-
>  3 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 14ad780a79..2f690a5b8e 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -20,6 +20,7 @@
>  
>  #include <string.h>
>  
> +#include "avassert.h"
>  #include "avstring.h"
>  #include "dict.h"
>  #include "dict_internal.h"
> @@ -38,6 +39,24 @@ int av_dict_count(const AVDictionary *m)
>      return m ? m->count : 0;
>  }
>  
> +AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
> +                                   const AVDictionaryEntry *prev)
> +{
> +    int i = 0;
> +
> +    if (!m)
> +        return NULL;
> +
> +    if (prev)
> +        i = prev - m->elems + 1;
> +
> +    av_assert2(i >= 0);
> +    if (i >= m->count)
> +        return NULL;
> +
> +    return &m->elems[i];
> +}
> +
>  AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
>                                 const AVDictionaryEntry *prev, int flags)
>  {
> diff --git a/libavutil/dict.h b/libavutil/dict.h
> index 0d1afc6c64..b42b3f07fd 100644
> --- a/libavutil/dict.h
> +++ b/libavutil/dict.h
> @@ -32,6 +32,8 @@
>  
>  #include <stdint.h>
>  
> +#include "attributes.h"
> +
>  /**
>   * @addtogroup lavu_dict AVDictionary
>   * @ingroup lavu_data
> @@ -101,6 +103,31 @@ typedef struct AVDictionary AVDictionary;
>  AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
>                                 const AVDictionaryEntry *prev, int flags);
>  
> +/**
> + * Iterate over a dictionary
> + *
> + * Iterates through all entries in the dictionary.
> + * 
> + * @warning The returned AVDictionaryEntry key/value must not be changed.
> + *
> + * @param m     The dictionary to iterate over
> + * @param prev  Pointer to the previous AVDictionaryEntry, NULL initially
> + * 
> + * @retval AVDictionaryEntry* The next element in the dictionary
> + * @retval NULL               No more elements in the dictionary
> + *
> + * Typical usage:
> + * @code
> + * AVDictionaryEntry *e = NULL;
> + * while (e = av_dict_iterate(m, e)) {
> + *     // ...
> + * }
> + * @endcode
> + */
> +av_warn_unused_result
> +AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
> +                                   const AVDictionaryEntry *prev);

The user is not allowed to modify the returned AVDictionaryEntries, so
you should return a const AVDictionaryEntry here. And there is no reason
to use av_warn_unused_result at all; nothing bad happens if you ignore
the result (except that you called av_dict_iterate unnecessarily).

Furthermore, av_dict_set's documentation contains "Adding a new entry to
a dictionary invalidates all existing entries previously returned with
av_dict_get." This needs to be updated, too.

> +
>  /**
>   * Get number of entries in dictionary.
>   *
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 0585fa7b80..6fd07ed2a4 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -80,7 +80,7 @@
>  
>  #define LIBAVUTIL_VERSION_MAJOR  57
>  #define LIBAVUTIL_VERSION_MINOR  36
> -#define LIBAVUTIL_VERSION_MICRO 102
> +#define LIBAVUTIL_VERSION_MICRO 103

New API additions need a minor bump (and need to reset micro).

>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>                                                 LIBAVUTIL_VERSION_MINOR, \