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.
