d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix some parsing and error handling bugs.

Browse Source 
r146736 added an undocumented syntax and many bugs handling it.  The
documented syntax is "... [mode] [fg [bg]] [show]", where it is critical
for reducing ambiguity and keeping things simple that the mode is
parsed first.  r146736 added buggy support for "... [mode] [fg [bg]]
[show] [mode] [fg [bg]]".  One error was that after for failing to set
a partially-supported graphics mode, argv[optind] remains pointing to
the mode so doesn't match the first [fg [bg]], so the setting is
attempted again, with slightly worse error handling.

Fix this by removing it (support for the trailing '[mode] [fg [bg]]')
and cleaning up.  The cleanups are mostly to remove convolutions and
bugs that didn't work to handle the ambiguous syntax '[fg [bg]] [fg [bg]]'
when [mode] and [show] are not present.  Globals were set to allow
repeating the color settings at the end.  The functions that set the
colors earlier were misnamed from set* to get*.  All that they "got" is
is settings from argv.  They applied the settings to the kernel and
the globals.

Fix restoration of colors in revert() by restoring 2 after the mode
change.  Colors should not need to be restored, but a bug in scteken
clobbers them on any mode change, including ones for restoration.  Don't
move the restoration of the other 3.  Teken doesn't clobber them on
mode changes because it doesn't support them at all (sc still supports
the border color, but only using a non-teken ioctl).

Add restoration of colors after a successful mode change to work around
the scteken bug there too.  The bug was previously masked by the general
setting of colors at the end.

Fix a longstanding parsing/error handling bug by exiting almost immediately
after matching the [mode] arg but failing to set the mode.  Just revert
if necessary.  Don't return to continue parsing but do it wrong.  This
bug caused spamming the output with a usage() message and exiting with
status 1 whenever [mode] is not present bug [fg [bg]] or [show].  The
exit code 1 was actualy an ambiguous internal code for failure to match
[mode] or failure to set [mode].  This 1 was obfuscated by spelling it
EXIT_FAILURE, but actual exit codes spell EXIT_FAILURE as 1.  Remove
another global which could have been used to disambiguate this but was
only used to micro-optimize the (unnecessary except for other bugs)
setting of colors at the end.
This commit is contained in:
Bruce Evans 2017-04-03 06:52:02 +00:00
parent db3b3ec5b4
commit 1b8c842e06
1 changed files with 24 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
usr.sbin/vidcontrol/vidcontrol.c
View File

@ -94,10 +94,6 @@ static int hex = 0;
static int vesa_cols;
static int vesa_rows;
static int font_height;
static int colors_changed;
static int video_mode_changed;
static int normal_fore_color, normal_back_color;
static int revers_fore_color, revers_back_color;
static int vt4_mode = 0;
static struct vid_info info;
static struct video_info new_mode_info;
@ -140,11 +136,6 @@ init(void)
if (ioctl(0, CONS_MODEINFO, &cur_info.video_mode_info) == -1)
errc(1, errno, "getting video mode parameters");
normal_fore_color = cur_info.console_info.mv_norm.fore;
normal_back_color = cur_info.console_info.mv_norm.back;
revers_fore_color = cur_info.console_info.mv_rev.fore;
revers_back_color = cur_info.console_info.mv_rev.back;
}
@ -163,8 +154,6 @@ revert(void)
ioctl(0, VT_ACTIVATE, cur_info.active_vty);
fprintf(stderr, "\033[=%dA", cur_info.console_info.mv_ovscan);
fprintf(stderr, "\033[=%dF", cur_info.console_info.mv_norm.fore);
fprintf(stderr, "\033[=%dG", cur_info.console_info.mv_norm.back);
fprintf(stderr, "\033[=%dH", cur_info.console_info.mv_rev.fore);
fprintf(stderr, "\033[=%dI", cur_info.console_info.mv_rev.back);
@ -185,6 +174,10 @@ revert(void)
ioctl(0, KDRASTER, size);
}
/* Restore some colors last since mode setting forgets some. */
fprintf(stderr, "\033[=%dF", cur_info.console_info.mv_norm.fore);
fprintf(stderr, "\033[=%dG", cur_info.console_info.mv_norm.back);
}
@ -657,7 +650,7 @@ set_cursor_type(char *appearance)
* Set the video mode.
*/
static int
static void
video_mode(int argc, char **argv, int *mode_index)
{
static struct {
@ -728,10 +721,11 @@ video_mode(int argc, char **argv, int *mode_index)
}
if (modes[i].name == NULL)
return EXIT_FAILURE;
return;
if (ioctl(0, mode, NULL) < 0) {
warn("cannot set videomode");
return EXIT_FAILURE;
ioerr = errno;
revert();
errc(1, ioerr, "cannot set videomode");
}
}
@ -805,16 +799,19 @@ video_mode(int argc, char **argv, int *mode_index)
else
ioctl(0, _IO('S', cur_mode), NULL);
revert();
warnc(ioerr, "cannot activate raster display");
return EXIT_FAILURE;
errc(1, ioerr,
"cannot activate raster display");
}
}
video_mode_changed = 1;
/* Recover from mode setting forgetting colors. */
fprintf(stderr, "\033[=%dF",
cur_info.console_info.mv_norm.fore);
fprintf(stderr, "\033[=%dG",
cur_info.console_info.mv_norm.back);
(*mode_index)++;
}
return EXIT_SUCCESS;
}
@ -836,66 +833,46 @@ get_color_number(char *color)
/*
* Get normal text and background colors.
* Set normal text and background colors.
*/
static void
get_normal_colors(int argc, char **argv, int *_index)
set_normal_colors(int argc, char **argv, int *_index)
{
int color;
if (*_index < argc && (color = get_color_number(argv[*_index])) != -1) {
(*_index)++;
fprintf(stderr, "\033[=%dF", color);
normal_fore_color=color;
colors_changed = 1;
if (*_index < argc
&& (color = get_color_number(argv[*_index])) != -1) {
(*_index)++;
fprintf(stderr, "\033[=%dG", color);
normal_back_color=color;
}
}
}
/*
* Get reverse text and background colors.
* Set reverse text and background colors.
*/
static void
get_reverse_colors(int argc, char **argv, int *_index)
set_reverse_colors(int argc, char **argv, int *_index)
{
int color;
if ((color = get_color_number(argv[*(_index)-1])) != -1) {
fprintf(stderr, "\033[=%dH", color);
revers_fore_color=color;
colors_changed = 1;
if (*_index < argc
&& (color = get_color_number(argv[*_index])) != -1) {
(*_index)++;
fprintf(stderr, "\033[=%dI", color);
revers_back_color=color;
}
}
}
/*
* Set normal and reverse foreground and background colors.
*/
static void
set_colors(void)
{
fprintf(stderr, "\033[=%dF", normal_fore_color);
fprintf(stderr, "\033[=%dG", normal_back_color);
fprintf(stderr, "\033[=%dH", revers_fore_color);
fprintf(stderr, "\033[=%dI", revers_back_color);
}
/*
* Switch to virtual terminal #arg.
*/
@ -1342,7 +1319,6 @@ main(int argc, char **argv)
char *font, *type, *termmode;
const char *opts;
int dumpmod, dumpopt, opt;
int reterr;
vt4_mode = is_vt4();
@ -1435,7 +1411,7 @@ main(int argc, char **argv)
dumpmod = DUMP_FMT_TXT;
break;
case 'r':
get_reverse_colors(argc, argv, &optind);
set_reverse_colors(argc, argv, &optind);
break;
case 'S':
set_lockswitch(optarg);
@ -1461,25 +1437,19 @@ main(int argc, char **argv)
if (dumpmod != 0)
dump_screen(dumpmod, dumpopt);
reterr = video_mode(argc, argv, &optind);
get_normal_colors(argc, argv, &optind);
video_mode(argc, argv, &optind);
set_normal_colors(argc, argv, &optind);
if (optind < argc && !strcmp(argv[optind], "show")) {
test_frame();
optind++;
}
video_mode(argc, argv, &optind);
if (termmode != NULL)
set_terminal_mode(termmode);
get_normal_colors(argc, argv, &optind);
if (colors_changed || video_mode_changed)
set_colors();
if ((optind != argc) || (argc == 1))
usage();
return reterr;
return (0);
}