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

[Daniel P. Berrange]

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.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|