[pulseaudio-discuss] [PATCH 5/5] pactl: Implement list message-handlers

[Tanu Kaskinen]

On Mon, 2018-01-22 at 16:19 +0100, Georg Chini wrote:
> On 21.01.2018 01:03, Tanu Kaskinen wrote:
> > It looks like we're anyway going to
> > need a bunch of "helper" functions, so there's not that much difference
> > between our approaches, mainly list parsing details are affected
> > somewhat. How to organize the helper functions is an open question,
> > since you have so far opposed my pa_message idea, and haven't provided
> > a proper alternative for keeping parameter writing state.
> 
> Can you perhaps explain your idea again? Maybe I just opposed
> because I did not understand what you proposed. I agree that we
> need helper functions to read and write values and if you like to
> include the braces around the parameter in the helper functions,
> I can live with it as well. What do you mean by "keeping parameter
> writing state"?

I mean the state that pa_strbuf keeps. Apparently you want to make
pa_strbuf available in libpulse and add helper functions in a different
namespace that nevertheless operates on pa_strbuf. You never explained
that explicitly, so it was not clear to me that you wanted that.

I'm ok with that plan. I would prefer pa_message_params as the helper
function namespace rather than just pa_message.

> > How should reading and writing nulls work? Writing nulls is not
> > complicated, we could just have pa_message_write_null() (or since we
> > probably will have a "raw string append" function, it's trivial to just
> > append "{}" manually). Reading nulls is less obvious.
> > 
> > All reading functions could have an "is_null" parameter:
> > 
> >      double double_var;
> >      int is_null;
> >      ret = pa_message_read_double(msg, &double_var, &is_null);
> > 
> > If the is_null parameter isn't provided (i.e. is itself NULL), the
> > function could fail if it encounters null.
> 
> If we define that NULL is represented by the "special element"
> {} it is very easy to determine if an element is empty, because
> in this case, the length returned by the parsing function will be 0.
> So I don't think we need to include an allow_null parameter in
> the helper function (unless we treat a sequence of white spaces
> like an empty element), because in cases where it is important,
> we can check the length of the element before calling the helper.
> Currently, parsing a value (including full error check) would look
> something like
> 
> ret = pa_split_message_parameter_string(list, max_length, &start_pos, 
> &length, NULL, &state);
> if (ret == 0)
>      handle empty element;
> if (ret == -1)
>      handle parse error;
> 
> /* start_pos contains now the first valid character of the element
> * after { and length the length of the element without closing
> * bracket */
> 
> ret = pa_message_read_double(start_pos, length, &double_var)
> 
> 
> Strings would not need a (read) helper function because they can
> be directly returned by the parsing function.

So you're proposing that reading a double should take two function
calls. I think the two steps should be merged into one
pa_message_params_read_double() function. Then error handling needs to
be done only once, rather than twice per value. With this approach,
however, the question arises how to deal with nulls.

> > Alternatively, if it's too ugly to have the is_null parameter in all
> > reading functions, there could be separate "_or_null()" functions
> > alongside the regular functions, e.g. pa_message_read_double_or_null().
> > 
> > And still one more topic: we might want to support arbitrary binary
> > data at some point. This could be supported by escaping non-ascii bytes
> > with \xnn (with n being a hexadecimal number). This escaping support
> > should exist from the beginning. Do we perhaps want to support \n, \r
> > and \t as well?
> > 
> 
> I do not think this must be supported from the start. "Binary data"
> would be a different data type than "string" and have special
> helper functions that can handle escaping.
> 
> This is different from escaping the control characters.

You're right. First I thought that the splitter function will get
confused if it encounters escape codes that it doesn't understand, but
unknown escape codes are fine, as long as they don't affect the message
structure.

-- 
Tanu

https://liberapay.com/tanuk
https://www.patreon.com/tanuk