Date: Thu, 21 Jan 2016 15:27:44 +0000 (UTC) From: Steven Hartland <smh@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r294506 - head/sys/boot/common Message-ID: <201601211527.u0LFRiXv069054@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: smh Date: Thu Jan 21 15:27:44 2016 New Revision: 294506 URL: https://svnweb.freebsd.org/changeset/base/294506 Log: Prevent loader.conf load failure due to unknown console entries When processing loader.conf if console contained an entry for an unsupported console then cons_set would return an error refusing to set any console. This has two side effects: 1. Forth would throw a syntax error and stop processing loader.conf at that point. 2. The value of console is ignored. #1 Means other important loader.conf entries may not be processed, which is clearly undesirable. #2 Means the users preference for console aren't applied even if they did contain valid options. Now we have support for multi boot paths from a single image e.g. bios and efi mode the console preference needs to deal with the need to set preference for more than one source. Fix this by: * Returning CMD_OK where possible from cons_set. * Allowing set with at least one valid console to proceed. Reviewed by: allanjude MFC after: 1 week Sponsored by: Multiplay Differential Revision: https://reviews.freebsd.org/D5018 Modified: head/sys/boot/common/console.c Modified: head/sys/boot/common/console.c ============================================================================== --- head/sys/boot/common/console.c Thu Jan 21 14:57:45 2016 (r294505) +++ head/sys/boot/common/console.c Thu Jan 21 15:27:44 2016 (r294506) @@ -38,7 +38,7 @@ __FBSDID("$FreeBSD$"); static int cons_set(struct env_var *ev, int flags, const void *value); static int cons_find(const char *name); static int cons_check(const char *string); -static void cons_change(const char *string); +static int cons_change(const char *string); static int twiddle_set(struct env_var *ev, int flags, const void *value); /* @@ -47,12 +47,12 @@ static int twiddle_set(struct env_var *e * as active. Also create the console variable. */ void -cons_probe(void) +cons_probe(void) { int cons; int active; char *prefconsole; - + /* We want a callback to install the new value when this var changes. */ env_setenv("twiddle_divisor", EV_VOLATILE, "1", twiddle_set, env_nounset); @@ -162,54 +162,69 @@ cons_find(const char *name) static int cons_set(struct env_var *ev, int flags, const void *value) { - int cons; + int ret; - if ((value == NULL) || (cons_check(value) == -1)) { - if (value != NULL) - printf("no such console!\n"); - printf("Available consoles:\n"); - for (cons = 0; consoles[cons] != NULL; cons++) - printf(" %s\n", consoles[cons]->c_name); - return(CMD_ERROR); + if ((value == NULL) || (cons_check(value) == 0)) { + /* + * Return CMD_OK instead of CMD_ERROR to prevent forth syntax error, + * which would prevent it processing any further loader.conf entries. + */ + return (CMD_OK); } - cons_change(value); + ret = cons_change(value); + if (ret != CMD_OK) + return (ret); env_setenv(ev->ev_name, flags | EV_NOHOOK, value, NULL, NULL); - return(CMD_OK); + return (CMD_OK); } /* - * Check that all of the consoles listed in *string are valid consoles + * Check that at least one the consoles listed in *string is valid */ static int cons_check(const char *string) { - int cons; + int cons, found, failed; char *curpos, *dup, *next; dup = next = strdup(string); - cons = -1; + found = failed = 0; while (next != NULL) { curpos = strsep(&next, " ,"); if (*curpos != '\0') { cons = cons_find(curpos); - if (cons == -1) - break; + if (cons == -1) { + printf("console %s is invalid!\n", curpos); + failed++; + } else { + found++; + } } } free(dup); - return (cons); + + if (found == 0) + printf("no valid consoles!\n"); + + if (found == 0 || failed != 0) { + printf("Available consoles:\n"); + for (cons = 0; consoles[cons] != NULL; cons++) + printf(" %s\n", consoles[cons]->c_name); + } + + return (found); } /* - * Activate all of the consoles listed in *string and disable all the others. + * Activate all the valid consoles listed in *string and disable all others. */ -static void +static int cons_change(const char *string) { - int cons; + int cons, active; char *curpos, *dup, *next; /* Disable all consoles */ @@ -219,6 +234,7 @@ cons_change(const char *string) /* Enable selected consoles */ dup = next = strdup(string); + active = 0; while (next != NULL) { curpos = strsep(&next, " ,"); if (*curpos == '\0') @@ -227,14 +243,37 @@ cons_change(const char *string) if (cons >= 0) { consoles[cons]->c_flags |= C_ACTIVEIN | C_ACTIVEOUT; consoles[cons]->c_init(0); - if ((consoles[cons]->c_flags & (C_PRESENTIN | C_PRESENTOUT)) != - (C_PRESENTIN | C_PRESENTOUT)) - printf("console %s failed to initialize\n", - consoles[cons]->c_name); + if ((consoles[cons]->c_flags & (C_PRESENTIN | C_PRESENTOUT)) == + (C_PRESENTIN | C_PRESENTOUT)) { + active++; + continue; + } + + if (active != 0) { + /* If no consoles have initialised we wouldn't see this. */ + printf("console %s failed to initialize\n", consoles[cons]->c_name); + } } } free(dup); + + if (active == 0) { + /* All requested consoles failed to initialise, try to recover. */ + for (cons = 0; consoles[cons] != NULL; cons++) { + consoles[cons]->c_flags |= C_ACTIVEIN | C_ACTIVEOUT; + consoles[cons]->c_init(0); + if ((consoles[cons]->c_flags & + (C_PRESENTIN | C_PRESENTOUT)) == + (C_PRESENTIN | C_PRESENTOUT)) + active++; + } + + if (active == 0) + return (CMD_ERROR); /* Recovery failed. */ + } + + return (CMD_OK); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201601211527.u0LFRiXv069054>