On Fri, Dec 04, 2015 at 04:04:11PM -0800, Philip Guenther wrote: > [...] For example, if the new code wants to call > endusershell(), then change PROTO_DEPRECATED(endusershell) to > PROTO_NORMAL(endusershell) and add a DEF_WEAK(endusershell) in the .c > file which defines it.
Applied that one, which makes the code easier to read again. Thank you for explaining what PROTO_DEPRECATED means, too. Fortunately I've discovered an issue with this code before it got into the tree: xdm(1) fails because it does not call setusershell() before getusershell(). This is actually no bug in xdm, because the manual page of getusershell(3) states: The getusershell() function reads the next line (opening the file if necessary); setusershell() rewinds the file; [...] This means that assert() must be removed and replaced with a call to setusershell() to (re)open the file. As the index variable might have changed, I've added a goto instruction so it'll be checked with the switch-statement again. Also adjusted the manual page here to clarify that we really need absolute path entries, never relative ones. Tobias Index: share/man/man5/shells.5 =================================================================== RCS file: /cvs/src/share/man/man5/shells.5,v retrieving revision 1.8 diff -u -p -u -p -r1.8 shells.5 --- share/man/man5/shells.5 31 May 2007 19:19:58 -0000 1.8 +++ share/man/man5/shells.5 5 Dec 2015 10:06:14 -0000 @@ -43,14 +43,14 @@ file contains a list of valid shells ava A shell is a command line interpreter that reads user input and executes commands. For each shell a single line should be present, consisting of the -shell's path, relative to root. +shell's absolute path. .Pp A hash mark .Pq Sq # indicates the beginning of a comment; subsequent characters up to the end of the line are not interpreted by the routines which search the file. -Blank lines are also ignored. +Blank lines and relative paths are also ignored. .Pp The .Xr chpass 1 Index: lib/libc/gen/getusershell.c =================================================================== RCS file: /cvs/src/lib/libc/gen/getusershell.c,v retrieving revision 1.16 diff -u -p -u -p -r1.16 getusershell.c --- lib/libc/gen/getusershell.c 14 Sep 2015 16:09:13 -0000 1.16 +++ lib/libc/gen/getusershell.c 5 Dec 2015 10:06:14 -0000 @@ -1,131 +1,104 @@ -/* $OpenBSD: getusershell.c,v 1.16 2015/09/14 16:09:13 tedu Exp $ */ +/* $OpenBSD$ */ /* - * Copyright (c) 1985, 1993 - * The Regents of the University of California. All rights reserved. + * Copyright (c) 2015 Mike Frysinger <vap...@gentoo.org> * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of the University nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. * - * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -#include <sys/stat.h> - -#include <ctype.h> -#include <limits.h> #include <paths.h> -#include <stdint.h> #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <unistd.h> -/* - * Local shells should NOT be added here. They should be added in - * /etc/shells. - */ - -static char *okshells[] = { _PATH_BSHELL, _PATH_CSHELL, _PATH_KSHELL, NULL }; -static char **curshell, **shells, *strings; -static char **initshells(void); - -/* - * Get a list of shells from _PATH_SHELLS, if it exists. - */ -char * -getusershell(void) -{ - char *ret; - - if (curshell == NULL) - curshell = initshells(); - ret = *curshell; - if (ret != NULL) - curshell++; - return (ret); -} +/* These functions are not thread safe (by design), so globals are OK. */ +static FILE *shellfp; +static int okshell_idx; +static char *shellbuf; +static size_t buf_len; void endusershell(void) { - - free(shells); - shells = NULL; - free(strings); - strings = NULL; - curshell = NULL; + okshell_idx = 0; + if (shellfp != NULL) { + fclose(shellfp); + shellfp = NULL; + free(shellbuf); + shellbuf = NULL; + } } +DEF_WEAK(endusershell); void setusershell(void) { + /* + * We could rewind shellfp here, but we get smaller code this way. + * Keep in mind this is not a performance sensitive API. + */ + endusershell(); - curshell = initshells(); + if ((shellfp = fopen(_PATH_SHELLS, "rce")) == NULL) + okshell_idx = 1; } +DEF_WEAK(setusershell); -static char ** -initshells(void) +char * +getusershell(void) { - char **sp, *cp; - FILE *fp; - struct stat statb; - - free(shells); - shells = NULL; - free(strings); - strings = NULL; - if ((fp = fopen(_PATH_SHELLS, "re")) == NULL) - return (okshells); - if (fstat(fileno(fp), &statb) == -1) { - (void)fclose(fp); - return (okshells); + /* + * Handle the builtin case first. + * + * NB: we do not use a global array here as the initialization needs + * relocations. These interfaces are used so rarely that this is not + * justified. Instead open code it with a switch statement. + */ +check_idx: + switch (okshell_idx) { + case 0: + /* Open file if required. */ + if (shellfp == NULL) { + setusershell(); + goto check_idx; + } + break; + case 1: + okshell_idx++; + return (char *) _PATH_BSHELL; + case 2: + okshell_idx++; + return (char *) _PATH_CSHELL; + case 3: + okshell_idx++; + return (char *) _PATH_KSHELL; + case 4: + return NULL; } - if (statb.st_size > SIZE_MAX) { - (void)fclose(fp); - return (okshells); - } - if ((strings = malloc((size_t)statb.st_size)) == NULL) { - (void)fclose(fp); - return (okshells); - } - shells = calloc((size_t)(statb.st_size / 3 + 2), sizeof (char *)); - if (shells == NULL) { - (void)fclose(fp); - free(strings); - strings = NULL; - return (okshells); - } - sp = shells; - cp = strings; - while (fgets(cp, PATH_MAX + 1, fp) != NULL) { - while (*cp != '#' && *cp != '/' && *cp != '\0') - cp++; - if (*cp == '#' || *cp == '\0') - continue; - *sp++ = cp; - while (!isspace((unsigned char)*cp) && *cp != '#' && *cp != '\0') - cp++; - *cp++ = '\0'; + + /* Walk the /etc/shells file. */ + while (getline(&shellbuf, &buf_len, shellfp) != -1) { + /* Strip out any comments and trailing newline. */ + shellbuf[strcspn(shellbuf, "#\n")] = '\0'; + + /* + * Only accept valid shells + * (which we define as starting with a '/'). + */ + if (shellbuf[0] == '/') + return shellbuf; } - *sp = NULL; - (void)fclose(fp); - return (shells); + + /* No more valid shells. */ + return NULL; } Index: lib/libc/hidden/unistd.h =================================================================== RCS file: /cvs/src/lib/libc/hidden/unistd.h,v retrieving revision 1.5 diff -u -p -u -p -r1.5 unistd.h --- lib/libc/hidden/unistd.h 17 Oct 2015 20:22:08 -0000 1.5 +++ lib/libc/hidden/unistd.h 5 Dec 2015 10:06:14 -0000 @@ -38,7 +38,7 @@ PROTO_NORMAL(crypt_newhash); PROTO_NORMAL(dup); PROTO_NORMAL(dup2); PROTO_NORMAL(dup3); -PROTO_DEPRECATED(endusershell); +PROTO_NORMAL(endusershell); PROTO_NORMAL(execl); PROTO_DEPRECATED(execle); PROTO_DEPRECATED(execlp); @@ -141,7 +141,7 @@ PROTO_NORMAL(setresuid); PROTO_NORMAL(setreuid); PROTO_NORMAL(setsid); PROTO_NORMAL(setuid); -PROTO_DEPRECATED(setusershell); +PROTO_NORMAL(setusershell); /*PROTO_CANCEL(sleep);*/ PROTO_DEPRECATED(strtofflags); PROTO_DEPRECATED(swab);