wrong comment in libpq.h

On 03.05.24 00:37, David Zhang wrote:

Hi Hackers,

There is a comment like below in src/include/libpq/libpq.h,

 /*
  * prototypes for functions in be-secure.c
  */
extern PGDLLIMPORT char *ssl_library;
extern PGDLLIMPORT char *ssl_cert_file;

...

However, 'ssl_library', 'ssl_cert_file' and the rest are global
parameter settings, not functions. To address this confusion, it would
be better to move all global configuration settings to the proper
section, such as /* GUCs */, to maintain consistency.

Maybe it's easier if we just replaced

prototypes for functions in xxx.c

with

declarations for xxx.c

throughout src/include/libpq/libpq.h.

On 3 May 2024, at 13:48, Peter Eisentraut <peter@eisentraut.org> wrote:

Maybe it's easier if we just replaced

prototypes for functions in xxx.c

with

declarations for xxx.c

throughout src/include/libpq/libpq.h.

+1

--
Daniel Gustafsson

On 2024-05-03 4:53 a.m., Daniel Gustafsson wrote:

On 3 May 2024, at 13:48, Peter Eisentraut <peter@eisentraut.org> wrote:
Maybe it's easier if we just replaced

prototypes for functions in xxx.c

with

declarations for xxx.c

throughout src/include/libpq/libpq.h.

+1

+1

--
Daniel Gustafsson

David

On 04.05.24 00:29, David Zhang wrote:

On 2024-05-03 4:53 a.m., Daniel Gustafsson wrote:

On 3 May 2024, at 13:48, Peter Eisentraut <peter@eisentraut.org> wrote:
Maybe it's easier if we just replaced

    prototypes for functions in xxx.c

with

    declarations for xxx.c

throughout src/include/libpq/libpq.h.

+1

+1

It looks like this wording "prototypes for functions in" is used many
times in src/include/, in many cases equally inaccurately, so I would
suggest creating a more comprehensive patch for this.

It looks like this wording "prototypes for functions in" is used many
times in src/include/, in many cases equally inaccurately, so I would
suggest creating a more comprehensive patch for this.

I noticed this "prototypes for functions in" in many header files and
briefly checked them. It kind of all make sense except the bufmgr.h has
something like below.

/* in buf_init.c */
extern void InitBufferPool(void);
extern Size BufferShmemSize(void);

/* in localbuf.c */
extern void AtProcExit_LocalBuffers(void);

/* in freelist.c */

which doesn't say "prototypes for functions xxx", but it still make
sense for me.

The main confusion part is in libpq.h.

/*
 * prototypes for functions in be-secure.c
 */
extern PGDLLIMPORT char *ssl_library;
extern PGDLLIMPORT char *ssl_cert_file;
extern PGDLLIMPORT char *ssl_key_file;
extern PGDLLIMPORT char *ssl_ca_file;
extern PGDLLIMPORT char *ssl_crl_file;
extern PGDLLIMPORT char *ssl_crl_dir;
extern PGDLLIMPORT char *ssl_dh_params_file;
extern PGDLLIMPORT char *ssl_passphrase_command;
extern PGDLLIMPORT bool ssl_passphrase_command_supports_reload;
#ifdef USE_SSL
extern PGDLLIMPORT bool ssl_loaded_verify_locations;
#endif

If we can delete the comment and move the variables declarations to /*
GUCs */ section, then it should be more consistent.

/* GUCs */
extern PGDLLIMPORT char *SSLCipherSuites;
extern PGDLLIMPORT char *SSLECDHCurve;
extern PGDLLIMPORT bool SSLPreferServerCiphers;
extern PGDLLIMPORT int ssl_min_protocol_version;
extern PGDLLIMPORT int ssl_max_protocol_version;

One more argument for my previous patch is that with this minor change
it can better align with the parameters in postgresql.conf.

# - SSL -

#ssl = off
#ssl_ca_file = ''
#ssl_cert_file = 'server.crt'
#ssl_crl_file = ''
#ssl_crl_dir = ''
#ssl_key_file = 'server.key'
#ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL'    # allowed SSL ciphers
#ssl_prefer_server_ciphers = on
#ssl_ecdh_curve = 'prime256v1'
#ssl_min_protocol_version = 'TLSv1.2'
#ssl_max_protocol_version = ''
#ssl_dh_params_file = ''
#ssl_passphrase_command = ''
#ssl_passphrase_command_supports_reload = off

best regards,

David

On Fri, May 10, 2024 at 11:14:51AM -0700, David Zhang wrote:

It looks like this wording "prototypes for functions in" is used many
times in src/include/, in many cases equally inaccurately, so I would
suggest creating a more comprehensive patch for this.

I noticed this "prototypes for functions in" in many header files and
briefly checked them. It kind of all make sense except the bufmgr.h has
something like below.

/* in buf_init.c */
extern void InitBufferPool(void);
extern Size BufferShmemSize(void);

/* in localbuf.c */
extern void AtProcExit_LocalBuffers(void);

/* in freelist.c */

which doesn't say "prototypes for functions xxx", but it still make sense
for me.

I looked at this and decided the GUC section was illogical, so I just
moved the variables up into the be-secure.c section. Patch attached.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"

On 17 Oct 2024, at 04:45, Bruce Momjian <bruce@momjian.us> wrote:

I looked at this and decided the GUC section was illogical, so I just
moved the variables up into the be-secure.c section. Patch attached.

No objections.

--
Daniel Gustafsson

On Thu, Oct 17, 2024 at 02:24:33PM +0200, Daniel Gustafsson wrote:

On 17 Oct 2024, at 04:45, Bruce Momjian <bruce@momjian.us> wrote:

I looked at this and decided the GUC section was illogical, so I just
moved the variables up into the be-secure.c section. Patch attached.

No objections.

Patch applied to master.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"