On 5/19/24 7:48 PM, Collin Funk wrote:
> It looks like the current code wants drive-prefixes accepted, i.e.
> 'c:/ugly/windows/stuff'.
>
> That sort of breaks the behavior of glibc and BSD where:
>
> input -> getusershell () output
> 'bin/bash' -> '/bash'
>
> Since it seems silly to accept some drive prefix in the middle of a
> string.
>
> I can't imagine anyone putting non-absolute file names in that file.
> So I'll probably restrict it to one absolute file name per-line and
> then trim any leading whitespace plus comments. Perhaps getline and
> filename.h macros will help there.
I've attached a patch following this method.
It should behave the same as BSD and glibc except for the fact that it
allows whitespace in file names. I guess you could scan the line
before returning on non-Windows systems, but I'm not sure it is worth
caring about. Only the root user can edit /etc/shells and programs
should check it actually exists anyways.
The patch removes 'GNULIB_GETUSERSHELL_SINGLE_THREAD' which uses
getc_unlocked instead of getc for optimization so I will wait for
review before pushing.
I don't see too much harm in it though since on most platforms getline
should lock much less then repeatedly calling getc. But I figured it
would be rude not to ask first. :)
Collin
From 192fa52c6efcc290fecb12c00fd69bd9f57c4b49 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.fu...@gmail.com>
Date: Sun, 19 May 2024 21:40:25 -0700
Subject: [PATCH] getusershell: Split file by lines instead of spaces.
* lib/getusershell.c: Include string.h and filename.h.
(GNULIB_GETUSERSHELL_SINGLE_THREAD): Remove conditional to include
unlocked stdio functions that are no longer used.
(readname): Remove function.
(next_shell): New function, based on readname.
(getusershell): Use next_shell instead of readname.
* doc/multithread.texi (Multithreading Optimizations): Don't mention
GNULIB_GETUSERSHELL_SINGLE_THREAD.
---
ChangeLog | 12 ++++++++
doc/multithread.texi | 4 ---
lib/getusershell.c | 69 ++++++++++++++++++++++++--------------------
modules/getusershell | 3 +-
4 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 89c5aa6993..ffe772bf2c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2024-05-19 Collin Funk <collin.fu...@gmail.com>
+
+ getusershell: Split file by lines instead of spaces.
+ * lib/getusershell.c: Include string.h and filename.h.
+ (GNULIB_GETUSERSHELL_SINGLE_THREAD): Remove conditional to include
+ unlocked stdio functions that are no longer used.
+ (readname): Remove function.
+ (next_shell): New function, based on readname.
+ (getusershell): Use next_shell instead of readname.
+ * doc/multithread.texi (Multithreading Optimizations): Don't mention
+ GNULIB_GETUSERSHELL_SINGLE_THREAD.
+
2024-05-19 Collin Funk <collin.fu...@gmail.com>
getusershell: Work around musl bugs.
diff --git a/doc/multithread.texi b/doc/multithread.texi
index 7d8126e8f3..424e1d2a75 100644
--- a/doc/multithread.texi
+++ b/doc/multithread.texi
@@ -293,10 +293,6 @@ @node Multithreading Optimizations
You can get this macro defined by including the Gnulib module
@code{wchar-single}.
@item
-You may define the C macro @code{GNULIB_GETUSERSHELL_SINGLE_THREAD}, if all the
-programs in your package invoke the functions @code{setusershell},
-@code{getusershell}, @code{endusershell} only from a single thread.
-@item
You may define the C macro @code{GNULIB_EXCLUDE_SINGLE_THREAD}, if all the
programs in your package invoke the functions of the @code{exclude} module
only from a single thread.
diff --git a/lib/getusershell.c b/lib/getusershell.c
index 4da5bff37f..a146dbfe89 100644
--- a/lib/getusershell.c
+++ b/lib/getusershell.c
@@ -34,16 +34,14 @@
#endif
#include <stdlib.h>
+#include <string.h>
#include <ctype.h>
#include "stdio--.h"
+#include "filename.h"
#include "xalloc.h"
-#if GNULIB_GETUSERSHELL_SINGLE_THREAD
-# include "unlocked-io.h"
-#endif
-
-static idx_t readname (char **, idx_t *, FILE *);
+static char *next_shell (char **, size_t *, FILE *);
#if ! defined ADDITIONAL_DEFAULT_SHELLS && defined __MSDOS__
# define ADDITIONAL_DEFAULT_SHELLS \
@@ -70,7 +68,7 @@ static FILE *shellstream = NULL;
static char *line = NULL;
/* Number of bytes allocated for 'line'. */
-static idx_t line_size = 0;
+static size_t line_size = 0;
/* Return an entry from the shells file, ignoring comment lines.
If the file doesn't exist, use the list in DEFAULT_SHELLS (above).
@@ -99,12 +97,7 @@ getusershell (void)
}
}
- while (readname (&line, &line_size, shellstream))
- {
- if (*line != '#')
- return line;
- }
- return NULL; /* End of file. */
+ return next_shell (&line, &line_size, shellstream);
}
/* Rewind the shells file. */
@@ -129,35 +122,47 @@ endusershell (void)
}
}
-/* Read a line from STREAM, removing any newline at the end.
+/* Read lines from STREAM, removing leading and trailing
+ whitespace and comments starting with the '#' character.
+ Finish reading lines once an absolute file name is found.
Place the result in *NAME, which is malloc'd
and/or realloc'd as necessary and can start out NULL,
and whose size is passed and returned in *SIZE.
- Return the number of bytes placed in *NAME
- if some nonempty sequence was found, otherwise 0. */
+ Return a pointer to the start of the file name which
+ may be in the middle of *NAME or NULL if the end-of-file
+ was reached. */
-static idx_t
-readname (char **name, idx_t *size, FILE *stream)
+static char *
+next_shell (char **name, size_t *size, FILE *stream)
{
- int c;
- size_t name_index = 0;
-
- /* Skip blank space. */
- while ((c = getc (stream)) != EOF && isspace (c))
- /* Do nothing. */ ;
-
for (;;)
{
- if (*size <= name_index)
- *name = xpalloc (*name, size, 1, -1, sizeof **name);
- if (c == EOF || isspace (c))
- break;
- (*name)[name_index++] = c;
- c = getc (stream);
+ ssize_t nbytes = getline (name, size, stream);
+ char *start;
+ char *end;
+ char *comment;
+
+ if (nbytes <= 0)
+ return NULL;
+
+ /* Trim the comment mark or trailing newline. */
+ start = *name;
+ comment = strchr (start, '#');
+ end = comment ? comment : start + nbytes - 1;
+ *end-- = '\0';
+
+ /* Skip leading whitespace. */
+ while (start < end && isspace ((unsigned char) *start))
+ start++;
+ /* Trim trailing whitespace. */
+ while (start < end && isspace ((unsigned char) *end))
+ *end-- = '\0';
+
+ /* Only return absolute file names. */
+ if (start < end && IS_ABSOLUTE_FILE_NAME (start))
+ return start;
}
- (*name)[name_index] = '\0';
- return name_index;
}
#ifdef TEST
diff --git a/modules/getusershell b/modules/getusershell
index 2f47c62e3d..a4a40852ac 100644
--- a/modules/getusershell
+++ b/modules/getusershell
@@ -9,7 +9,8 @@ Depends-on:
unistd
extensions
fopen-safer [test $HAVE_GETUSERSHELL = 0 || test $REPLACE_GETUSERSHELL = 1]
-unlocked-io-internal [test $HAVE_GETUSERSHELL = 0 || test $REPLACE_GETUSERSHELL = 1]
+getline [test $HAVE_GETUSERSHELL = 0 || test $REPLACE_GETUSERSHELL = 1]
+filename [test $HAVE_GETUSERSHELL = 0 || test $REPLACE_GETUSERSHELL = 1]
xalloc [test $HAVE_GETUSERSHELL = 0 || test $REPLACE_GETUSERSHELL = 1]
configure.ac:
--
2.45.1
