[Spice-devel] [PATCH spice-gtk] channel-main: Convert text line-endings if necessary (rhbz#752350)

[Hans de Goede]

Hi,

On 06/24/2013 03:25 PM, Daniel P. Berrange wrote:
> On Mon, Jun 24, 2013 at 03:23:30PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 06/24/2013 03:04 PM, Daniel P. Berrange wrote:
>>> On Mon, Jun 24, 2013 at 02:45:25PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 06/24/2013 02:39 PM, Daniel P. Berrange wrote:
>>>>> On Mon, Jun 24, 2013 at 02:31:27PM +0200, Hans de Goede wrote:
>>>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>>>> ---
>>>>>>   gtk/channel-main.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>>>>>>   1 file changed, 69 insertions(+), 4 deletions(-)
>>>>>
>>>>> IIUC, implicit in your patch here is that the communication protocol
>>>>> between spice client & guest agent uses guest OS specific line ending
>>>>> markers, and the spice client must do all conversions to guest OS
>>>>> format. Is that really what we want ?
>>>>
>>>> Yes, this is by design. The reason for this is that there are no real
>>>> conventions as to what line-endings to use inside the clipboard data,
>>>> so ie the clipboard on unix (X) may contain text data with CRLF
>>>> line-endings. And some programs may depend on this.
>>>>
>>>> So to minimize the change of breaking things we don't want to do any
>>>> conversion when doing copy and paste between a unix guest and unix client,
>>>> or a windows guest and windows client.
>>>>
>>>>> Personally I would have said that the line ending markers should be
>>>>> invariant (always \n) in the comms protocol and that each end point
>>>>> must handle conversion to/from \r\n if required.
>>>>
>>>> That was my initial design too, but see above.
>>>>
>>>>> But perhaps there are backwards compat issues forcing your choice
>>>>> here ?
>>>>
>>>> Well currently we don't do any conversions, so keeping the wire
>>>> format in guest "encoding" makes things a bit easier wrt backward
>>>> compat handling, but we could have moved to always use \n on the
>>>> wire using guest capabilities bits, so that is not the main reason
>>>> for this.
>>>
>>> Ok, that makes sense.
>>>
>>> Would be nice to describe this rationale in the commit message for
>>> benefit of people looking back at this change years down the road.
>>
>> It is already described in the commit message of the protocol update
>> for this, which is IMHO the proper place for this:
>> http://cgit.freedesktop.org/spice/spice-protocol/commit/?id=7be0e88e7e03a956b364cc847aad11b96ed47273
>
> IMHO it should still be described here, or that commit could be
> referenced here as the source of the policy being implemented.

Ok, I've added a reference to the spice-protocol commit to the commit
msg for this in my local tree.

Regards,

Hans