Hi Collin,

> I've attached a patch following this method.

I see a method 'next_shell' that is only called in a single place. Would
it make sense to inline it (and move its function comments into the
body of the caller function)?

The main change is to call getline() instead of parsing the file
character-by-character. I guess that the motivation is that the
behaviour of trimming leading whitespace and trimming trailing whitespace
before '#' would lead to convoluted code with the previous approach?

> The patch removes 'GNULIB_GETUSERSHELL_SINGLE_THREAD' which uses
> getc_unlocked instead of getc for optimization ...
> 
> I don't see too much harm in it though since on most platforms getline
> should lock much less then repeatedly calling getc.

Right. When the function uses getline instead of getc, it will lock 10x
less than before. I agree there is not much need to optimize this.
Especially since we don't have a getline_unlocked function. And since
locking has become cheaper since this code was written. (Futexes were
invented in 2002-2003.)

Bruno




Reply via email to