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

Wed Jan 24 01:10:33 UTC 2018

On Mon, 2018-01-22 at 21:43 +0100, Georg Chini wrote:
> On 21.01.2018 01:03, Tanu Kaskinen wrote:
> > On Fri, 2018-01-19 at 14:23 +0100, Georg Chini wrote:
> > > Also you need to handle the case where the end of a list does not
> > > match the end of the string because there are further elements
> > > behind it.
> > 
> > Why? There can't be any further elements behind the implicit list. As I
> > said earlier, the drawback with the implicit list is that the message
> > parameters aren't extensible. If the list is explicit, then it's
> > possible to detect when the list ends and other parameters begin.
> 
> This is what I mean. Only the outer list is implicit. So it is not
> enough to look for end of string but you need also test if you
> hit an "end of list". If you have nested lists, it may be possible
> that there are elements behind the end of the list you are
> currently working on. Concerning the "implicit outer list":
> Adding additional braces around the whole parameter list is
> up to the message handler. If you prefer, I can do this for the
> core message handler, though I believe it is not necessary in
> that case. The implicit list is not a design problem, it does not
> change anything if you think it through. It's just that "start of
> string" and "end of string" are used instead of braces.

Are you claiming that if the "list message handlers" call returns an
implicit list, i.e. no braces around the outermost list, then it's
still possible to later add a new parameter after the list? I'm saying
that's impossible, and that's the problem with the implicit list. How
can the parsing code detect when the list ends and the new parameter
begins, if originally "end of string" was the only indication that the
list ended?

> > > > > 2) You are forced to process the elements in the order they are delivered,
> > > > > taking care of all the nesting. This has two disadvantages:
> > > > > 
> > > > > a) Because you do not know if the nesting brackets are closed correctly, you
> > > > > may continue processing elements after a missing bracket. Consider the
> > > > > following example: {{x1} {x2} {x3}} {y1} {y2}. If you forget to close the
> > > > > list like {{x1} {x2} {x3} {y1} {y2}, your approach would continue processing
> > > > > y1 and y2 as part of the list before the error is encountered.
> > > > > 
> > > > > split_message_response() however would complain before any of the elements
> > > > > is processed because it searches for the closing bracket and thereby does a
> > > > > consistency check of each element that is returned.
> > > > 
> > > > You're right, but I don't see why this is such a bad thing. The error
> > > > will be caught eventually.
> > > 
> > > If you only parse the data it is not an issue but if you are already
> > > using the
> > > values in the parsing loop this might cause problems. Let me make a (not
> > > very realistic) example: We have a 5.1 output and the x1 to x3 are used
> > > to change the volume of the first three channels. Now you would also
> > > change the volume of the 4th and 5th channel to some random value
> > > before you hit an error.
> > 
> > Your approach doesn't actually help here. pa_split_message_response()
> > can't do much more than check that the braces match. It doesn't know
> > the types of the contained values, so the values can still be invalid
> > even if pa_split_message_response() thinks everything is fine.
> 
> Well, the values themselves cannot be checked, but that can't
> be done for both solutions. But my way avoids that values are
> used for something unintended if a parse error occurs. The
> point in the case above is that my function will find the parse
> error before the values are used, while your solution finds
> the parse error only after the values have been used.

How does validating the brace correctness prevent some values being
used before all values have been parsed? You're only enforcing brace
correctness, but if you have a list of doubles, any of the double
values can be invalid. Nothing is preventing the application from using
the first values of the list before noticing that a value later in the
list is invalid.

I didn't claim that my approach was any better in this regard, you were
just making impossible-sounding claims about the benefits of your
approach.

> > It would certainly be useful to be able to skip uninteresting values.
> > There should be a function for this.
> 
> You can simply use the parsing function to skip a value.

Sure, I was talking about my approach that didn't have a similar built-
in mechanism for this.

> > > > pa_context_send_message() would take a pa_message as an argument. The
> > > > reply callback would get a pa_message as an argument. Signal callbacks
> > > > would get a pa_message as an argument. So things would work very
> > > > similarly to how D-Bus works. One major difference to D-Bus would be
> > > > that errors wouldn't be messages, at least for now (earlier there was
> > > > some talk about making errors more informative, and at that point using
> > > > pa_message for representing errors as well might make sense).
> > > 
> > > Did we not agree that the client should only receive a string as a reply?
> > > If we can pass structures, we don't need all the overhead of transforming
> > > the variables into strings.
> > 
> > Even if we use a pa_message struct, it's still good to have the
> > protocol string-based. For example, that makes "pactl send-message"
> > possible (or at least much easier to use than with a binary protocol).
> 
> I tried to understand but I don't get what the advantage
> of such a structure is.

I introduced the pa_message structure as a solution to the problem that
we need something like pa_strbuf, but pa_strbuf isn't fully adequate as
it is.

Now it's starting to be clear that you want to use pa_strbuf plus some
separate helper functions. That's fine. The small benefit of pa_message
would be that the structure would contain both the necessary parts of
pa_strbuf and the helper functions all in the same namespace.

Another benefit would be that if pa_context_send_message() took a
pa_message rather than a string for the parameters, then applications
wouldn't need to first allocate a pa_strbuf, then convert that into a
string and then free the string. Using pa_message would have one less
step. The same benefit could be achieved if pa_context_send_message()
took a pa_strbuf instead of a string for the parameters, but I'm not
sure if we want to do that.

-- 
Tanu

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