Re: Thread-safe getopt()

Thanks for the review!

On 27/03/2026 16:29, Peter Eisentraut wrote:
> On 19.05.25 22:22, Heikki Linnakangas wrote:
>>> - getopt()
>>>
>>> This needs a custom replacement.  (There is no getopt_r() because
>>> programs usually don't call getopt() after startup.)
>>>
>>> (Note: This is also called during session startup, not only during
>>> initial postmaster start.  So we definitely need something here, if we
>>> want to, like, start more than one session concurrently.)
>>
>> Here's a patch for a thread-safe version of getopt(). I implemented it
>> as two functions, pg_getopt_start() and pg_getopt_next(). Since
>> there's no standard to follow, we have freedom on how to implement it,
>> and IMHO that feels more natural than the single getopt() function. I
>> took the implementation from the getopt() replacement we already had
>> for platforms that don't have getopt(), moving all the global
>> variables it used to a struct.
>>
>> The last patch attached replaces all calls in the server to use the
>> new variant, but leaves all the calls in client programs alone. I
>> considered changing the client programs as well, but there's no
>> immediate need, and it seems nice to use OS functions when possible.
>>
>> (The first patch fixes a little harmless bug in
>> get_stats_option_name() that's gone unnoticed since 2006 but got in
>> the way now.)
>
> That first patch seems like a genuine latent bug that should be fixed.
>
> The API you have created here looks pretty good to me.
>
> I don't think we need to apply this to BootstrapModeMain() or
> PostmasterMain(), it would be sufficient to use it in
> process_postgres_switches() in tcop/postgres.c.  It don't see any
> advantage in using it where not needed (let alone client programs).

I'd prefer to change those too. It's nice to be able to say "there are
no getopt() calls in the backend binary", and not have to scrutinize
which ones are OK. Also I actually like the new API better than plain
getopt().

Agreed on the client programs.

> (I don't suppose there is a way to get rid of the need to do command-
> line option parsing for the startup packet.  It seems to be too widely
> used.)

Right..

Attached is a rebased version of the patches, no material changes.
Barring objections, I'll commit later tonight or tomorrow.

- Heikki