From owner-svn-src-head@freebsd.org Mon Apr 3 06:52:04 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2ABA6D2BB24; Mon, 3 Apr 2017 06:52:04 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DE75B7C2; Mon, 3 Apr 2017 06:52:03 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v336q2cu028418; Mon, 3 Apr 2017 06:52:02 GMT (envelope-from bde@FreeBSD.org) Received: (from bde@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v336q21V028417; Mon, 3 Apr 2017 06:52:02 GMT (envelope-from bde@FreeBSD.org) Message-Id: <201704030652.v336q21V028417@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: bde set sender to bde@FreeBSD.org using -f From: Bruce Evans Date: Mon, 3 Apr 2017 06:52:02 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r316443 - head/usr.sbin/vidcontrol X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Apr 2017 06:52:04 -0000 Author: bde Date: Mon Apr 3 06:52:02 2017 New Revision: 316443 URL: https://svnweb.freebsd.org/changeset/base/316443 Log: Fix some parsing and error handling bugs. 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. Modified: head/usr.sbin/vidcontrol/vidcontrol.c Modified: head/usr.sbin/vidcontrol/vidcontrol.c ============================================================================== --- head/usr.sbin/vidcontrol/vidcontrol.c Mon Apr 3 06:14:23 2017 (r316442) +++ head/usr.sbin/vidcontrol/vidcontrol.c Mon Apr 3 06:52:02 2017 (r316443) @@ -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 *m } 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 *m 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,67 +833,47 @@ 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); }