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);

Reply via email to