Implement "vidcontrol -h <history_size>" for vt(4)

[guest-svmhdvn]

Summary

Fixes bug 210415

[ray]

Looks good. Thanks.

sys/dev/vt/vt_buf.c

553 ↗

(On Diff #31450)

Idea to use function, to have way offload it to hardware.
We still don't support hw offload, but maybe :)

[emaste]

usr.sbin/vidcontrol/vidcontrol.c

1296–1298 ↗

(On Diff #31450)

The error string certainly didn't agree with the previous size < 0 test, but is vidcontrol -h 0 actually not permissible -- is it not possible to specify no history?

[guest-svmhdvn]

usr.sbin/vidcontrol/vidcontrol.c

1296–1298 ↗

(On Diff #31450)

It is technically possible to specify no history, but it will have the same effect as running vidcontrol -h <x> for all x <= screen_height. From sys/dev/vt/vt_buf.c:

void
vtbuf_sethistory_size(struct vt_buf *vb, unsigned int size)
{
	term_pos_t p;

	/* With same size */
	p.tp_row = vb->vb_scr_size.tp_row;
	p.tp_col = vb->vb_scr_size.tp_col;
	vtbuf_grow(vb, &p, size);
}

void
vtbuf_grow(struct vt_buf *vb, const term_pos_t *p, unsigned int history_size)
{
	term_char_t *old, *new, **rows, **oldrows, **copyrows, *row, *oldrow;
	unsigned int w, h, c, r, old_history_size;
	size_t bufsize, rowssize;
	int history_full;

	history_size = MAX(history_size, p->tp_row);
	...
}

Since history_size will be at least screen_height large, should it be permissible to specify a history size of 0 or be consistent with the previous error string?

[emaste]

usr.sbin/vidcontrol/vidcontrol.c

1296–1298 ↗

(On Diff #31450)

Hmm, good point, although it might be useful to allow -h 0 to mean minimum history size.

[emaste]

sys/dev/vt/vt_core.c

2209 ↗

(On Diff #31450)

ah, further to my comment on vidcontrol.c for vidcontrol -h 0 to mean minimal history we'd need to make this < 0

[emaste]

sys/dev/vt/vt_core.c

2209 ↗

(On Diff #31450)

Also we'll want some maximum on the history size in order to prevent excessive memory allocation for the buffer.

[guest-svmhdvn]

sys/dev/vt/vt_core.c

2209 ↗

(On Diff #31450)

syscons(4) enforces the maximum on the history size and returns EINVAL if the check fails, so it would make sense to do a similar check in vt(4) rather than one in the vidcontrol(1) tool right?

[emaste]

sys/dev/vt/vt_core.c

2209 ↗

(On Diff #31450)

Yes, the check should be in the kernel; if it were only in vidcontrol someone could build a vidcontrol(1) without the check. We might want to return ERANGE instead though, it seems to be a better fit and would allow a better error message to be returned to the user.

(This may not be the exact intended use of ERANGE according to POSIX but for FreeBSD-specific ioctls we have some freedom to use errnos as we see fit.)

This test can be combined with the existing by just casting to an unsigned int, removing the need to test for < 0.

[guest-svmhdvn]

• Add a check for the maximum history size. This maximum is set to the same value as the maximum in syscons(4).
• Fix the error string on a negative number argument

[emaste]

In my wipbsd-12 branch as 8838eabfacb87fbf3781d9a5a79de88ffb859215