Page MenuHomeFreeBSD

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

Authored by smahadevan_freebsdfoundation.org on Aug 1 2017, 10:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 20, 1:20 AM
Unknown Object (File)
Fri, Mar 1, 11:05 AM
Unknown Object (File)
Jan 30 2024, 3:44 AM
Unknown Object (File)
Jan 11 2024, 5:48 AM
Unknown Object (File)
Dec 20 2023, 7:45 AM
Unknown Object (File)
Dec 16 2023, 3:41 AM
Unknown Object (File)
Dec 14 2023, 12:06 AM
Unknown Object (File)
Dec 11 2023, 12:15 AM
Subscribers

Details

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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 :)

This revision is now accepted and ready to land.Aug 2 2017, 9:32 AM
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?

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?

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.

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

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.

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?

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.

smahadevan_freebsdfoundation.org edited edge metadata.
  • 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
This revision now requires review to proceed.Aug 15 2017, 4:25 PM

In my wipbsd-12 branch as 8838eabfacb87fbf3781d9a5a79de88ffb859215

This revision was automatically updated to reflect the committed changes.