This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 19 Aug 2015 09:28, Paul Pluzhnikov wrote: > On Wed, Aug 19, 2015 at 7:55 AM, Mike Frysinger wrote: > > /* 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; > > > > char * > > getusershell (void) > > { > > /* 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. */ > > switch (okshell_idx) > > { > > case 0: > > /* Require the user call setusershell first. */ > > assert (shellfp != NULL); > > I could find no man page that requires user calling setusershell first. > Is there a definitive description of this interface? hmm, looks like freebsd, openbsd, darwin, and gnulib don't require this. i'll drop that. > > while (getline (&shellbuf, &buf_len, shellfp) != -1) > > { > > char *p; > > > > /* Strip out any comment lines. */ > > p = strchr (shellbuf, '#'); > > if (p) > > *p = '\0'; > > else > > { > > /* Chop the trailing newline. */ > > p = strchr (shellbuf, '\n'); > > Current version chops on white space. Do we want to return "/foo bar" > here? Of course any sysadmin who puts "/foo bar" into /etc/shells gets > what he deserves. the old logic is even worse than that. it skips all leading bytes until it finds a slash and then returns things after that, while splitting on space. so a line like: space/foobar cow will return "/foobar". the docs i've seen (darwin, freebsd, linux) leave it fairly vague and say "valid user shell". but no one validates things, and really only document comments and blank lines. i think we should just go with that. > > /* Only accept valid shells (which we define as starting with a '/'). */ > > if (shellbuf[0] == '/') > > Do we want to return "/" as a valid shell here? the current already does :). since we aren't checking the path in the file (i.e. running stat and seeing if it's a +x file), i don't think we want to treat / specially. afterall, "/bogus" or "/bin" or "/sbin/" will still get returned. > > 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 (); > > > > shellfp = fopen (_LOCAL_PATH_SHELLS, "rce"); > > if (shellfp == NULL) > > okshell_idx = 1; > > I think you missed a part here: > > else > okshell_idx = 4; nope ... current code is correct. see how the value of 0 is treated (which is also the default since it's a bss var). -mike
/* Copyright (C) 2015 Free Software Foundation, Inc. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. The GNU C Library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ #include <assert.h> #include <paths.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> /* Used by tests to point to custom paths. */ #ifndef _LOCAL_PATH_SHELL # define _LOCAL_PATH_SHELLS _PATH_SHELLS #endif /* 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; char * getusershell (void) { /* 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. */ switch (okshell_idx) { case 0: if (shellfp == NULL) { setusershell (); return getusershell (); } break; case 1: okshell_idx++; return (char *) _PATH_BSHELL; case 2: okshell_idx++; return (char *) _PATH_CSHELL; case 3: return NULL; } /* Walk the /etc/shells file. */ while (1) { ssize_t len; char *p; len = getline (&shellbuf, &buf_len, shellfp); if (len <= 0) break; /* Strip out any comment lines. */ p = strchr (shellbuf, '#'); if (p) *p = '\0'; else { /* Chop the trailing newline. */ p = shellbuf + len - 1; if (*p == '\n') *p = '\0'; } /* Only accept valid shells (which we define as starting with a '/'). */ if (shellbuf[0] == '/') return shellbuf; } /* No more valid shells. */ return NULL; } hidden_proto (endusershell); void endusershell (void) { okshell_idx = 0; if (shellfp) { fclose (shellfp); shellfp = NULL; free (shellbuf); shellbuf = NULL; } } hidden_def (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 (); shellfp = fopen (_LOCAL_PATH_SHELLS, "rce"); if (shellfp == NULL) okshell_idx = 1; }
Attachment:
signature.asc
Description: Digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |