Simplify Signature of ProcessStartupPacket()
Technical Problem
This thread addresses a code cleanup opportunity in PostgreSQL's connection startup path. The function ProcessStartupPacket() — which handles the initial protocol exchange when a client connects to the server — previously accepted ssl_done and gss_done boolean parameters. These parameters tracked whether SSL and GSS negotiation had already been completed during the startup handshake.
A prior commit (b63f25bddfeb) refactored the startup code such that ProcessStartupPacket() now has only one caller. When a function has a single call site, parameters that could instead be managed as local variables or within the function itself represent unnecessary interface complexity. The ssl_done and gss_done state can be internalized into the function body rather than being passed through the signature.
Why This Matters Architecturally
The connection startup path in PostgreSQL (postmaster.c / authentication handling) is one of the most security-sensitive code paths in the system. It handles:
- Protocol version negotiation
- SSL/TLS upgrade negotiation
- GSSAPI encryption negotiation
- Authentication method selection
Simplifying the interface of ProcessStartupPacket() provides several benefits:
- Reduced cognitive load: Developers working on connection security don't need to trace parameter passing through multiple layers.
- Encapsulation: The SSL/GSS negotiation state becomes an internal implementation detail rather than a caller's responsibility, which is the correct abstraction boundary when there's only one caller.
- Fewer opportunities for bugs: Eliminating parameters that could be incorrectly passed reduces the attack surface in security-critical code.
Design Decision
The approach is straightforward: move the ssl_done and gss_done variables from being function parameters to being local variables within ProcessStartupPacket(). This is enabled entirely by the fact that the prior refactoring in b63f25bddfeb reduced the number of callers from multiple to one, eliminating the original reason for parameterization (different callers needed to pass different state).
Version Targeting Discussion
A minor discussion point was whether this should target only HEAD (v20) or also be backpatched to v19. Michael Paquier initially proposed it for HEAD/v20 only, being conservative. Heikki Linnakangas suggested it could go into v19, reasoning that as a pure code simplification with no behavioral change, the risk is negligible. Michael agreed, though the final application was to HEAD only (consistent with the original proposal and the typical conservatism around backpatching even trivial changes to released branches).
Outcome
The patch was applied to HEAD after receiving quick consensus (+1) from both Heikki Linnakangas and Daniel Gustafsson (who originally suggested the cleanup during the review of commit b63f25bddfeb).