[PATCH v2] misc: Optimize getusershell.c

[Collin Funk]

Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:

> On 22/04/26 14:01, Collin Funk wrote:
>> Rocket Ma <marocketbd@gmail.com> writes:
>> 
>>> * misc/getusershell.c: Completely rewrite the unit. Only allocate one
>>> big buffer to store shell names. Add a missing unit test.
>>>
>>> The new implementation read the whole file into one buffer, and wipe out
>>> every byte but shell names. Later when addressing shell names from first
>>> shell, jump to next '\0' and then jump to next '/'. This could reduce
>>> memory footprint and shall improve some performance.
>>>
>>> Signed-off-by: Rocket Ma <marocketbd@gmail.com>
>>> ---
>>> This new patch fixed some unexpected plt links, and use isspace_l to
>>> force parsing /etc/shells with C locale. (Is that necessary?)
>> 
>> Thanks for the patch.
>> 
>> However, I am against this change. The /etc/shells file can be
>> arbitrarily long, so we shouldn't read it all into memory. It also
>> changes the behavior of the function to be different to all the BSD
>> versions. In this version appending a shell while the process is running
>> will not affect subsequent calls to getusershell.
>
> The current code already read the whole file before parsing it, this
> new approach is no worse.  The BSD code also seems to limit each
> shell to PATH_MAX, while this approach seems to work in-place and
> removes the extra buffer allocation.

Ah, thanks to you and Rocket for pointing out my mistake. I think I was
mixing up the one in glibc and the one I wrote for Gnulib to work around
some issues. Sorry for the extra noise.

The one in Gnulib uses getline, mostly because it was easier. I guess
the downside to that is that you might read a very long line which would
be impossible to execute as a shell, instead of limiting things to
PATH_MAX. It looks like OpenBSD decided to go in that direction as well
[1].

Collin

[1] https://github.com/openbsd/src/blob/7a2e62a4900cc0c671c264dfa2679f73987d45c6/lib/libc/gen/getusershell.c#L103