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

[Hans de Goede]

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

Regards,

Hans