Re: [PATCH v3] console: Add persistent scrollback buffers for all VGA consoles

Hi,

I finally found the time to rework the patches, but there are a few
things I want to discuss:

quoted

This breaks tools like clear_console that rely on flushing the
scrollback history by switching back and forth between consoles
which is why this feature is disabled by default.
Use the escape sequence \e[3J instead for flushing the buffer.

Have never heard of such tool as clear_console. Why mention it at all,
where is it used, which are practical configurations which are broken in
this way?

clear_console(1) is part of Debian's bash package and (I think) a
inofficial 'downstream' patch developed by Ubuntu [1]. There has been
security concerns (which is also why I disabled this feature by
default).
However, if there might be security issues, I would strongly suggest to
keep this part in the documentation - it does no harm anyway, does it?

Does this break clear(1)?

No, my tests with clear(1) worked fine.

quoted

-         Enter the amount of System RAM to allocate for the scrollback
-	 buffer.  Each 64KB will give you approximately 16 80x25
-	 screenfuls of scrollback buffer
+	  Enter the amount of System RAM to allocate for scrollback
+	  buffers of VGA consoles. Each 64KB will give you approximately
+	  16 80x25 screenfuls of scrollback buffer.

Indentation amended, I don't know which way it should be, please
double-check or mimic old indentation to stay on safe side.

Double-checked! That's at least how I read the docs.

quoted

+static struct vgacon_scrollback_info *vgacon_scrollback_cur;

Could you use name vgacon_scrollback instead of new
vgacon_scrollback_cur? It doesn't change anything conceptually, but is a
bit shorter and seems to mean current scrollback context anyway, right?

I think I see nothing to comment on below.

There is a already an array called 'vgacon_scrollbacks[N]', so I would
not like to have two variables named 'vgacon_scrollback' and
'vgacon_scrollbacks'. Either {'vgacon_scrollback_cur' and
'vgacon_scrollbacks'} or {'vgacon_scrollback' and
'vgacon_scrollback_list'}.
You can make a suggestion, if you like.

All other point you mentioned where fixed as you suggested.

Thanks again for your help, Andrey!

Best,

Manuel

[1]
http://manpages.ubuntu.com/manpages/xenial/en/man1/clear_console.1.html