Page MenuHomeFreeBSD

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

Authored by guest-svmhdvn on Aug 1 2017, 10:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 4, 5:14 AM
Unknown Object (File)
Wed, Dec 4, 5:14 AM
Unknown Object (File)
Wed, Dec 4, 5:14 AM
Unknown Object (File)
Wed, Dec 4, 5:13 AM
Unknown Object (File)
Wed, Dec 4, 4:53 AM
Unknown Object (File)
Sun, Dec 1, 2:18 AM
Unknown Object (File)
Fri, Nov 29, 4:57 PM
Unknown Object (File)
Oct 16 2024, 2:50 PM
Subscribers

Details

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

guest-svmhdvn added a subscriber: ray.
guest-svmhdvn removed a subscriber: ray.

Looks good. Thanks.

sys/dev/vt/vt_buf.c
553

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

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

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

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

sys/dev/vt/vt_core.c
2209

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

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

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

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 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.