[pulseaudio-discuss] [PATCH 1/3] modargs: Add a mechanism to append modargs

David Henningsson david.henningsson at canonical.com
Tue Nov 12 06:23:17 PST 2013


On 10/07/2013 05:39 PM, Arun Raghavan wrote:
> This allows us to parse an extra set of modargs to tack on to an
> existing set. Duplicates in the second set are ignored, since this fits
> our use best. In the future, this could be extended to support different
> merge modes (ignore dupes vs. replace with dupes), but I've left this
> out since there isn't a clear need and it would be dead code for now.

Fair point, but I think of an append function as replacing rather than
ignoring - maybe a better name would be pa_modargs_append_ignoredups()
or so?

> ---
>  src/pulsecore/modargs.c | 54 ++++++++++++++++++++++++++++++++-----------------
>  src/pulsecore/modargs.h |  2 ++
>  2 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/src/pulsecore/modargs.c b/src/pulsecore/modargs.c
> index 7665b91..ddaa54d 100644
> --- a/src/pulsecore/modargs.c
> +++ b/src/pulsecore/modargs.c
> @@ -45,7 +45,7 @@ struct entry {
>      char *key, *value;
>  };
>  
> -static int add_key_value(pa_modargs *ma, char *key, char *value, const char* const valid_keys[]) {
> +static int add_key_value(pa_modargs *ma, char *key, char *value, const char* const valid_keys[], bool ignore_dupes) {
>      struct entry *e;
>      char *raw;
>  
> @@ -58,7 +58,11 @@ static int add_key_value(pa_modargs *ma, char *key, char *value, const char* con
>      if (pa_hashmap_get(ma->unescaped, key)) {
>          pa_xfree(key);
>          pa_xfree(value);
> -        return -1;
> +
> +        if (ignore_dupes)
> +            return 0;
> +        else
> +            return -1;
>      }
>  
>      if (valid_keys) {
> @@ -102,7 +106,7 @@ static void free_func(void *p) {
>      pa_xfree(e);
>  }
>  
> -pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
> +static int pa_modargs_parse(pa_modargs *ma, const char *args, const char* const* valid_keys, bool ignore_dupes) {
>      enum {
>          WHITESPACE,
>          KEY,
> @@ -117,13 +121,6 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
>  
>      const char *p, *key = NULL, *value = NULL;
>      size_t key_len = 0, value_len = 0;
> -    pa_modargs *ma = pa_xnew(pa_modargs, 1);
> -
> -    ma->raw = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL, free_func);
> -    ma->unescaped = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL, free_func);
> -
> -    if (!args)
> -        return ma;
>  
>      state = WHITESPACE;
>  
> @@ -162,7 +159,8 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
>                      if (add_key_value(ma,
>                                        pa_xstrndup(key, key_len),
>                                        pa_xstrdup(""),
> -                                      valid_keys) < 0)
> +                                      valid_keys,
> +                                      ignore_dupes) < 0)
>                          goto fail;
>                      state = WHITESPACE;
>                  } else if (*p == '\\') {
> @@ -181,7 +179,8 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
>                      if (add_key_value(ma,
>                                        pa_xstrndup(key, key_len),
>                                        pa_xstrndup(value, value_len),
> -                                      valid_keys) < 0)
> +                                      valid_keys,
> +                                      ignore_dupes) < 0)
>                          goto fail;
>                      state = WHITESPACE;
>                  } else if (*p == '\\') {
> @@ -201,7 +200,8 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
>                      if (add_key_value(ma,
>                                        pa_xstrndup(key, key_len),
>                                        pa_xstrndup(value, value_len),
> -                                      valid_keys) < 0)
> +                                      valid_keys,
> +                                      ignore_dupes) < 0)
>                          goto fail;
>                      state = WHITESPACE;
>                  } else if (*p == '\\') {
> @@ -221,7 +221,8 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
>                      if (add_key_value(ma,
>                                        pa_xstrndup(key, key_len),
>                                        pa_xstrndup(value, value_len),
> -                                      valid_keys) < 0)
> +                                      valid_keys,
> +                                      ignore_dupes) < 0)
>                          goto fail;
>                      state = WHITESPACE;
>                  } else if (*p == '\\') {
> @@ -239,23 +240,40 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
>      }
>  
>      if (state == VALUE_START) {
> -        if (add_key_value(ma, pa_xstrndup(key, key_len), pa_xstrdup(""), valid_keys) < 0)
> +        if (add_key_value(ma, pa_xstrndup(key, key_len), pa_xstrdup(""), valid_keys, ignore_dupes) < 0)
>              goto fail;
>      } else if (state == VALUE_SIMPLE) {
> -        if (add_key_value(ma, pa_xstrndup(key, key_len), pa_xstrdup(value), valid_keys) < 0)
> +        if (add_key_value(ma, pa_xstrndup(key, key_len), pa_xstrdup(value), valid_keys, ignore_dupes) < 0)
>              goto fail;
>      } else if (state != WHITESPACE)
>          goto fail;
>  
> -    return ma;
> +    return 0;
>  
>  fail:
> +    return -1;
> +}
>  
> -    pa_modargs_free(ma);
> +pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
> +    pa_modargs *ma = pa_xnew(pa_modargs, 1);
> +
> +    ma->raw = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL, free_func);
> +    ma->unescaped = pa_hashmap_new_full(pa_idxset_string_hash_func, pa_idxset_string_compare_func, NULL, free_func);
> +
> +    if (args && pa_modargs_parse(ma, args, valid_keys, false) < 0)
> +        goto fail;
>  
> +    return ma;
> +
> +fail:
> +    pa_modargs_free(ma);
>      return NULL;
>  }
>  
> +int pa_modargs_append(pa_modargs *ma, const char *args, const char* const* valid_keys) {
> +    return pa_modargs_parse(ma, args, valid_keys, true);
> +}
> +
>  void pa_modargs_free(pa_modargs*ma) {
>      pa_assert(ma);
>  
> diff --git a/src/pulsecore/modargs.h b/src/pulsecore/modargs.h
> index 95be54d..5637493 100644
> --- a/src/pulsecore/modargs.h
> +++ b/src/pulsecore/modargs.h
> @@ -35,6 +35,8 @@ typedef struct pa_modargs pa_modargs;
>  
>  /* Parse the string args. The NULL-terminated array keys contains all valid arguments. */
>  pa_modargs *pa_modargs_new(const char *args, const char* const keys[]);
> +/* Parse the string args, and add any keys that are not already present. */
> +int pa_modargs_append(pa_modargs *ma, const char *args, const char* const* valid_keys);

It seems a bit inconsistent that modargs_new takes a "const char* const
keys[]" whereas pa_modargs_append takes a "const char* const*
valid_keys" (i e, one has an extra [] and the other an extra *).

Care to steer this up so it's the same everywhere?

Another option could be to store a pointer to valid_keys in
pa_modargs_new and just use that pointer in modargs_append. But I wonder
if we dynamically create this array somewhere and destroy it afterwards,
if so, it's not a good idea to store this pointer...


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic


More information about the pulseaudio-discuss mailing list