On 26.07.2018 12:37, Tanu Kaskinen wrote: > On Sun, 2018-07-22 at 21:11 +0200, Georg Chini wrote: >> On 22.07.2018 17:48, Tanu Kaskinen wrote: >>> On Sun, 2018-07-22 at 16:02 +0200, Georg Chini wrote: >>>> On 21.07.2018 20:17, Tanu Kaskinen wrote: >>>>> On Mon, 2018-04-09 at 19:35 +0200, Georg Chini wrote: >>>>>> The patch adds the possibility to escape curly braces within parameter strings >>>>>> and introduces several new functions that can be used for writing parameters. >>>>>> >>>>>> For writing, the structure pa_message_param, which is a wrapper for pa_strbuf >>>>>> has been created. Following new write functions are available: >>>>>> >>>>>> pa_message_param_new() - creates a new pa_message_param structure >>>>>> pa_message_param_to_string() - converts a pa_message_param to string and frees >>>>>> the structure >>>>>> The function pa_message_param_write_string() >>>>>> has a parameter do_escape. >>>>> Why not do escaping always? >>>> Because what you are writing as a string may be a list that you have >>>> prepared >>>> previously. Then you will not want escaping. You may for example create >>>> a list >>>> from an array and then insert this list as one string into the final >>>> parameter list >>>> or have a function that converts a certain structure to a parameter >>>> string and >>>> then write the result of this function as one element of the final list. >>> My mental model is that parameters have types, list type is different >>> than string type, and write_string() is only meant for writing values >>> of the string type. >>> >>> Can you add a write_raw() function? >> Yes, this is done in patch 7. But the raw write function differs from what >> write_string() is doing. write_string() writes one element of a list, >> that is >> it encloses the string in braces. The raw write function is intended for >> situations where you can't write a complete element with one write, so >> it does not add any braces. I am still of the opinion, that a structure >> or array converted to a parameter string is a string, so writing something >> like this should be done with write_string(). > They are different kinds of strings, different abstraction levels. When > you're writing an array "as a string", in that context it's just a C > string. write_string() with escaping deals with strings in the "message > params type system". I don't know if this makes any sense to you. > Probably not... In any case, the do_escape flag seems unnecessary > complexity to me. The alternative would be a function to write an unescaped string in addition to the write_raw() function. If you don't like the flag, would you be OK with a write_unescaped_string() function? I think it is just more comfortable than using write_raw(). > >> Also writing unescaped strings in situations where escaping is not necessary >> saves the overhead of looping over all the characters. >> >>>>> core-util already contains pa_unescape() that does the same thing more >>>>> efficiently (if you drop the single quote thing). >>>> pa_unescape() currently does not do the same thing. It removes all >>>> escape characters, while I only want to remove the characters >>>> I actually introduced (those before { or }). >>>> I can however modify pa_unescape() to take the same arguments >>>> as pa_escape(). >>> I don't see the need for being selective when unescaping. Nothing >>> breaks if all (except escaped) backslashes are stripped. >> You are right, if previously all backslashes in the original string >> have been escaped, nothing will break. I was still thinking of the >> old solution where I did not escape backslashes. >> >> >>>>>> + >>>>>> +/* Read functions */ >>>>>> + >>>>>> /* Split the specified string into elements. An element is defined as >>>>>> * a sub-string between curly braces. The function is needed to parse >>>>>> * the parameters of messages. Each time it is called returns the position >>>>>> * of the current element in result and the state pointer is advanced to >>>>>> - * the next list element. >>>>>> + * the next list element. On return, the parameter *is_unpacked indicates >>>>>> + * if the string is plain text or contains a sub-list. is_unpacked may >>>>>> + * be NULL. >>>>> is_unpacked looks like unnecessary complexity. >>>>> pa_message_params_read_string() should always unescape the value. >>>> It may be possible, that the string you read is a list. Consider the >>>> following >>>> parameter list: {string1}{some nested structure}{string2}. You can now >>>> read this list as three strings and then continue to read the elements of >>>> the nested structure from the second string. You might even create a >>>> function >>>> that takes a string and outputs a structure. So you are not forced to go >>>> to the full depth of nesting on the first pass. This makes it much easier >>>> to handle deeply nested parameter lists. For me this behavior is an >>>> important >>>> feature and I do not want to drop it. See also my comment on why I do >>>> not always want escaping. >>> Doesn't split_list() already allow this, why do you want to use >>> read_string() to do the same thing as split_list()? >> read_string() and split_list() are very similar and we could live >> without read_string(). It is provided as a counterpart to write_string() >> and for convenience additionally does the unescaping if necessary >> like write_string does the escaping. >> I don't see why this is a problem. It depends on the context which >> is the better function to use. > Again, in my mind a structure is not a string, they are different > types, and I think read_string() should only deal with the string type. > is_unpacked makes the API more complicated, so I'd like to get rid of > it. > I don't see your point. is_unpacked is not part of the read_string() arguments or return value. In split_list() it is definitively needed to indicate if the returned string (in the C sense) contains another list. I can imagine many situations where you might either pass an array or just a single value or even NULL. is_unpacked allows to differentiate between the situations. And I don't think that a boolean return value is adding significantly to the complexity of a function, especially since passing NULL is allowed if you do not care for the value.