On Mon, 2018-04-09 at 19:35 +0200, Georg Chini wrote:
> ---
>  doc/messaging_api.txt      |   5 ++
>  src/map-file               |   4 +
>  src/pulse/message-params.c | 202 
> +++++++++++++++++++++++++++++++++++++++++++++
>  src/pulse/message-params.h |  12 +++
>  4 files changed, 223 insertions(+)
> 
> diff --git a/doc/messaging_api.txt b/doc/messaging_api.txt
> index d080b783..57a92f58 100644
> --- a/doc/messaging_api.txt
> +++ b/doc/messaging_api.txt
> @@ -53,6 +53,11 @@ pa_message_param_read_double() - read a double from a 
> parameter list
>  pa_message_param_read_int64() - read an integer from a parameter list
>  pa_message_param_read_uint64() - read an unsigned from a parameter list
>  pa_message_param_read_bool() - read a boolean from a parameter list
> +pa_message_param_read_double_array() - read an array of double from list
> +pa_message_param_read_int64_array() - read an array of int64 from list
> +pa_message_param_read_uint64_array() - read an array of uint64 from list
> +pa_message_param_read_string_array() - read an array of strings from a list
> +
>  
>  All read functions return 1 on success, 0 if end of string is found and -1 on
>  parse error. Additionally, for the numeric/boolean read functions, 2 is 
> returned
> diff --git a/src/map-file b/src/map-file
> index ab8c21c6..88d892ef 100644
> --- a/src/map-file
> +++ b/src/map-file
> @@ -230,9 +230,13 @@ pa_message_param_end_list;
>  pa_message_param_new;
>  pa_message_param_read_bool;
>  pa_message_param_read_double;
> +pa_message_param_read_double_array;
>  pa_message_param_read_int64;
> +pa_message_param_read_int64_array;
>  pa_message_param_read_string;
> +pa_message_param_read_string_array;
>  pa_message_param_read_uint64;
> +pa_message_param_read_uint64_array;
>  pa_message_param_split_list;
>  pa_message_param_to_string;
>  pa_message_param_write_bool;
> diff --git a/src/pulse/message-params.c b/src/pulse/message-params.c
> index 93972399..b9846863 100644
> --- a/src/pulse/message-params.c
> +++ b/src/pulse/message-params.c
> @@ -38,6 +38,8 @@ struct pa_message_param {
>      pa_strbuf *buffer;
>  };
>  
> +/* Helper functions */
> +
>  /* Remove escaping from a string */
>  static char *unescape(char *value) {
>      char *tmp;
> @@ -62,6 +64,59 @@ static char *unescape(char *value) {
>      return value;
>  }
>  
> +/* Count number of top level elements in parameter list */
> +static int count_elements(const char *c) {
> +    const char *s;
> +    uint32_t element_count;
> +    bool found_element, found_backslash;
> +    int open_braces;
> +
> +    if (!c || *c == 0)
> +        return PA_PARAM_LIST_END;
> +
> +    element_count = 0;
> +    open_braces = 0;
> +    found_element = false;
> +    found_backslash = false;
> +    s = c;
> +
> +    /* Count elements in list */
> +    while (*s != 0) {
> +
> +        /* Skip escaped curly braces. */
> +        if (*s == '\\') {
> +            found_backslash = true;
> +            s++;
> +            continue;
> +        }
> +
> +        if (*s == '{' && !found_backslash) {
> +            found_element = true;
> +            open_braces++;
> +        }
> +        if (*s == '}' && !found_backslash)
> +            open_braces--;
> +
> +        /* unexpected closing brace, parse error */
> +        if (open_braces < 0)
> +            return PA_PARAM_PARSE_ERROR;
> +
> +        if (open_braces == 0 && found_element) {
> +            element_count++;
> +            found_element = false;
> +        }
> +
> +        found_backslash = false;
> +        s++;
> +    }
> +
> +    /* missing closing brace, parse error */
> +    if (open_braces > 0)
> +        return PA_PARAM_PARSE_ERROR;
> +
> +    return element_count;
> +}
> +
>  /* Read functions */
>  
>  /* Split the specified string into elements. An element is defined as
> @@ -300,6 +355,153 @@ int pa_message_param_read_bool(char *c, bool *result, 
> void **state) {
>      return PA_PARAM_OK;
>  }
>  
> +/* Converts a parameter list to a string array. Escaping is removed
> + * from a string if the string does not contain a list. If allocate
> + * is true, new strings will be allocated, otherwise pointers to
> + * sub-strings within c will be returned. */
> +int pa_message_param_read_string_array(char *c, char ***parameter_list, bool 
> allocate) {
> +    char *start_pos;
> +    void *state = NULL;
> +    uint32_t element_count, i;
> +    bool is_unpacked;
> +    int err;
> +
> +    pa_assert(parameter_list);
> +
> +    *parameter_list = NULL;

The return value should be set only on success (see the patch 7 review
for more elaborate explanation). This comment applies to the other
functions as well.

> +
> +    /* Count elements, return if no element was found or parse error. */
> +    if ((element_count = count_elements(c)) <= 0)
> +        return element_count;
> +
> +    /* Allocate array */
> +    *parameter_list = pa_xmalloc0(element_count * sizeof(char *));
> +
> +    i = 0;
> +    while ((err = pa_message_param_split_list(c, &start_pos, &is_unpacked, 
> &state)) > 0) {
> +        (*parameter_list)[i] = start_pos;
> +        if (is_unpacked)
> +            (*parameter_list)[i] = unescape(start_pos);
> +
> +        /* If new strings are allocated, they must be freed by the caller */
> +        if (allocate)
> +            (*parameter_list)[i] = pa_xstrdup((*parameter_list)[i]);
> +
> +        i++;
> +    }

I believe pa_message_params_read_string() can be used, and that should
result in simpler code.

> +
> +    if (err < 0) {
> +        if (allocate) {
> +            for (i = 0; i < element_count; i++)
> +                pa_xfree((*parameter_list)[i]);
> +        }
> +        pa_xfree(*parameter_list);
> +        *parameter_list = NULL;
> +        return PA_PARAM_PARSE_ERROR;
> +    }
> +
> +    return element_count;
> +}
> +
> +/* Converts a parameter list to a double array. */
> +int pa_message_param_read_double_array(char *c, double **parameter_list) {
> +    double  value;
> +    void *state = NULL;
> +    uint32_t element_count, i;
> +    int err;
> +
> +    pa_assert(parameter_list);
> +
> +    *parameter_list = NULL;
> +
> +    /* Count elements, return if no element was found or parse error. */
> +    if ((element_count = count_elements(c)) <= 0)
> +        return element_count;
> +
> +    /* Allocate array */
> +    *parameter_list = pa_xmalloc(element_count * sizeof(double));
> +
> +    i = 0;
> +    while ((err = pa_message_param_read_double(c, &value, &state)) > 0) {
> +       (*parameter_list)[i] = value;
> +        i++;
> +    }

The value variable is not needed, and maybe a for loop would be a
better fit. Maybe you did things this way because you think this is
more readable? I don't mind very much, so keep it as it is if you
prefer.

The loop could be rewritten like this:

    for (i = 0; (err = pa_message_param_read_double(c, &values[i], &state)) > 
0; i++)
        ;

I replaced &(*parameter_list)[i] with &values[i], because we should
modify parameter_list only on success, so here values is a local double
array variable.

This comment applies to the other functions as well.

> +
> +    if (err < 0) {
> +        pa_xfree(*parameter_list);
> +        *parameter_list = NULL;
> +        return PA_PARAM_PARSE_ERROR;
> +    }
> +
> +    return element_count;
> +}
> +
> +/* Converts a parameter list to an int64 array. */
> +int pa_message_param_read_int64_array(char *c, int64_t **parameter_list) {
> +    int64_t  value;
> +    void *state = NULL;
> +    uint32_t element_count, i;
> +    int err;
> +
> +    pa_assert(parameter_list);
> +
> +    *parameter_list = NULL;
> +
> +    /* Count elements, return if no element was found or parse error. */
> +    if ((element_count = count_elements(c)) <= 0)
> +        return element_count;
> +
> +    /* Allocate array */
> +    *parameter_list = pa_xmalloc(element_count * sizeof(int64_t));
> +
> +    i = 0;
> +    while ((err = pa_message_param_read_int64(c, &value, &state)) > 0) {
> +       (*parameter_list)[i] = value;
> +        i++;
> +    }
> +
> +    if (err < 0) {
> +        pa_xfree(*parameter_list);
> +        *parameter_list = NULL;
> +        return PA_PARAM_PARSE_ERROR;
> +    }
> +
> +    return element_count;
> +}
> +
> +/* Converts a parameter list to an uint64 array. */
> +int pa_message_param_read_uint64_array(char *c, uint64_t **parameter_list) {
> +    uint64_t  value;
> +    void *state = NULL;
> +    uint32_t element_count, i;
> +    int err;
> +
> +    pa_assert(parameter_list);
> +
> +    *parameter_list = NULL;
> +
> +    /* Count elements, return if no element was found or parse error. */
> +    if ((element_count = count_elements(c)) <= 0)
> +        return element_count;
> +
> +    /* Allocate array */
> +    *parameter_list = pa_xmalloc(element_count * sizeof(uint64_t));
> +
> +    i = 0;
> +    while ((err = pa_message_param_read_uint64(c, &value, &state)) > 0) {
> +       (*parameter_list)[i] = value;
> +        i++;
> +    }
> +
> +    if (err < 0) {
> +        pa_xfree(*parameter_list);
> +        *parameter_list = NULL;
> +        return PA_PARAM_PARSE_ERROR;
> +    }
> +
> +    return element_count;
> +}
> +
>  /* Write functions. The functions are wrapper functions around pa_strbuf,
>   * so that the client does not need to use pa_strbuf directly. */
>  
> diff --git a/src/pulse/message-params.h b/src/pulse/message-params.h
> index f865e236..3ab1e418 100644
> --- a/src/pulse/message-params.h
> +++ b/src/pulse/message-params.h
> @@ -60,6 +60,18 @@ int pa_message_param_read_uint64(char *c, uint64_t 
> *result, void **state);
>  /** Read a boolean from the parameter list. */
>  int pa_message_param_read_bool(char *c, bool *result, void **state);
>  
> +/** Convert message parameter list to string array */
> +int pa_message_param_read_string_array(char *c, char ***parameter_list, bool 
> allocate);
> +
> +/** Convert message parameter list to double array */
> +int pa_message_param_read_double_array(char *c, double **parameter_list);
> +
> +/** Convert message parameter list to int64 array */
> +int pa_message_param_read_int64_array(char *c, int64_t **parameter_list);
> +
> +/** Convert message parameter list to uint64 array */
> +int pa_message_param_read_uint64_array(char *c, uint64_t **parameter_list);
> +
>  /** Write functions */
>  
>  /** Create a new pa_message_param structure */
-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to