[8/8] message-params: Add read functions for arrays

Submitted by Georg Chini on April 9, 2018, 5:35 p.m.

Details

Message ID 20180409173525.18219-9-georg@chini.tk
State Superseded
Headers show
Series "core: Add message sending/receiving" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Georg Chini April 9, 2018, 5:35 p.m.
---
 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(+)

Patch hide | download patch | download mbox

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;
+
+    /* 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++;
+    }
+
+    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++;
+    }
+
+    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 */

Comments

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 */
On 11.08.2018 11:37, Tanu Kaskinen wrote:
> 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 */

Thank you for your reviews. It will probably take until October before I can
send a new series.
On Wed, 2018-08-15 at 21:31 +0200, Georg Chini wrote:
> Thank you for your reviews. It will probably take until October before I can
> send a new series.

OK, no problem!