Stefano Stabellini
2009-Oct-12 17:20 UTC
[Xen-devel] [PATCH 4 of 7] minios: fix minios console
MiniOS'' console_print tries to expand ''\n'' into "\n\r" in place, causing page faults if the string resides in text. Use a duplicate of the string instead. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- diff -r 21935dfa8e5f extras/mini-os/console/console.c --- a/extras/mini-os/console/console.c Fri Oct 09 19:30:39 2009 +0100 +++ b/extras/mini-os/console/console.c Mon Oct 12 16:03:49 2009 +0100 @@ -79,6 +79,8 @@ void console_print(struct consfront_dev *dev, char *data, int length) { char *curr_char, saved_char; + char copied_str[length]; + char *copied_ptr; int part_len; int (*ring_send_fn)(struct consfront_dev *dev, const char *data, unsigned length); @@ -86,24 +88,26 @@ ring_send_fn = xencons_ring_send_no_notify; else ring_send_fn = xencons_ring_send; - - for(curr_char = data; curr_char < data+length-1; curr_char++) + + copied_ptr = copied_str; + memcpy(copied_ptr, data, length); + for(curr_char = copied_ptr; curr_char < copied_ptr+length-1; curr_char++) { if(*curr_char == ''\n'') { saved_char = *(curr_char+1); *(curr_char+1) = ''\r''; - part_len = curr_char - data + 2; - ring_send_fn(dev, data, part_len); + part_len = curr_char - copied_ptr + 2; + ring_send_fn(dev, copied_ptr, part_len); *(curr_char+1) = saved_char; - data = curr_char+1; + copied_ptr = curr_char+1; length -= part_len - 1; } } - ring_send_fn(dev, data, length); + ring_send_fn(dev, copied_ptr, length); - if(data[length-1] == ''\n'') + if(copied_ptr[length-1] == ''\n'') ring_send_fn(dev, "\r", 1); } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mick Jordan
2009-Oct-12 22:54 UTC
Re: [Xen-devel] [PATCH 4 of 7] minios: fix minios console
On 10/12/09 10:20 AM, Stefano Stabellini wrote:> MiniOS'' console_print tries to expand ''\n'' into "\n\r" in place, causing > page faults if the string resides in text. > Use a duplicate of the string instead. >Are you sure this is even necessary? I removed this in my GUK extension of MiniOS with no apparent effect. The actual push to remove it was observing that when "piping" the output of one GUK (JVM) guest to another, the "\n\r" was being treated as two line feeds by the Java runtime. Why? Because it is "\r\n" that is historically reduced to "\n", and not "\n\r". In any event in the Xen/Unix environment there should be no need for the expansion at all. And it really helped to clean up the console code. Mick _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2009-Oct-13 12:22 UTC
Re: [Xen-devel] [PATCH 4 of 7] minios: fix minios console
On Mon, 12 Oct 2009, Mick Jordan wrote:> On 10/12/09 10:20 AM, Stefano Stabellini wrote: > > MiniOS'' console_print tries to expand ''\n'' into "\n\r" in place, causing > > page faults if the string resides in text. > > Use a duplicate of the string instead. > > > Are you sure this is even necessary? I removed this in my GUK extension > of MiniOS with no apparent effect. The actual push to remove it was > observing that when "piping" the output of one GUK (JVM) guest to > another, the "\n\r" was being treated as two line feeds by the Java > runtime. Why? Because it is "\r\n" that is historically reduced to "\n", > and not "\n\r". In any event in the Xen/Unix environment there should be > no need for the expansion at all. And it really helped to clean up the > console code. >The expansion of \n in \r\n is for consistency with linux ttys, but you are right about the fact that minios actually substitutes \n with \n\r, and this is definitely another bug. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mick Jordan
2009-Oct-13 16:34 UTC
Re: [Xen-devel] [PATCH 4 of 7] minios: fix minios console
On 10/13/09 05:22 AM, Stefano Stabellini wrote:> The expansion of \n in \r\n is for consistency with linux ttys, but you > are right about the fact that minios actually substitutes \n with \n\r, > and this is definitely another bug. > >So what does "consistency with linux ttys" actually mean in practice? I am running under Linux and am not seeing any issues without the conversion. Can you cite some situation where not doing the conversion would cause a problem? E.g., failing to start printing at the beginning of the line. [I assume no-one is using ASR-33''s any more!]. Mick _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2009-Oct-13 17:15 UTC
Re: [Xen-devel] [PATCH 4 of 7] minios: fix minios console
Mick Jordan writes ("Re: [Xen-devel] [PATCH 4 of 7] minios: fix minios console"):> So what does "consistency with linux ttys" actually mean in practice? I > am running under Linux and am not seeing any issues without the > conversion. Can you cite some situation where not doing the conversion > would cause a problem? E.g., failing to start printing at the beginning > of the line. [I assume no-one is using ASR-33''s any more!].Well, it''s like this: the Linux hvc driver (or its equivalent in 2.6.18 vintage kernels) gets the data after it has been processed by the general tty layer. In most cases (if the terminal is in the most normal mode) this means that \n written by applications has been already changed to \r\n. This is because ttys are (virtual or real) serial ports. Thus, in current practice with non-stubdom, the data over the Xen console ring is \r\n (and generally formatted as data you might send over a serial port). Thus the xen pv console is `supposed'' to work more like a virtual hardware serial line than like (say) a Unix pipe. This is not dealt with wholly consistently by the xen-unstable toolstack. For example the default behaviour of xenconsoled is to stuff the received data directly into a file; this is in some sense wrong because arguably the file ought to be a Unix text file (that is, a logfile) rather than a session transcript (as you might get out of `script''). But if you were to plumb the console directly to a hardware serial port or to another tty (eg one connected to a terminal emulator program), this lf => crlf conversion is essential. Without it you will get incorrect output on whatever is displaying the stream. Without a proper spec for all of this it''s hard to say that anything in particular is right or wrong but since the guest behaviour is hardest to change I would suggest we declare that to be correct. And if we do that, then we must also say that minios should behave like other guests and write cr-lf terminated lines, not lf-terminated ones. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel