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
