wrong comment in libpq.h
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.
I have attached an attempt to help address this issue.
Thank you,
David
Attachments:
0001-fix-the-wrong-comment-to-keep-the-consistency.patchtext/plain; charset=UTF-8; name=0001-fix-the-wrong-comment-to-keep-the-consistency.patchDownload
From 9154102c34ec0c4c956d25942b8ea600dd740a07 Mon Sep 17 00:00:00 2001
From: David Zhang <idrawone@gmail.com>
Date: Thu, 2 May 2024 15:15:13 -0700
Subject: [PATCH] fix the wrong comment to keep the consistency
---
src/include/libpq/libpq.h | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 83e338f604..bece50b69c 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -86,19 +86,6 @@ extern bool pq_check_connection(void);
/*
* 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
-
extern int secure_initialize(bool isServerStart);
extern bool secure_loaded_verify_locations(void);
extern void secure_destroy(void);
@@ -117,6 +104,19 @@ extern ssize_t secure_open_gssapi(Port *port);
#endif
/* GUCs */
+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
+
extern PGDLLIMPORT char *SSLCipherSuites;
extern PGDLLIMPORT char *SSLECDHCurve;
extern PGDLLIMPORT bool SSLPreferServerCiphers;
--
2.34.1
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 replacedprototypes 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 replacedprototypes 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?"
Attachments:
include.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 142c98462ed..2a1ff89eb71 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -86,19 +86,6 @@ extern bool pq_check_connection(void);
/*
* 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
-
extern int secure_initialize(bool isServerStart);
extern bool secure_loaded_verify_locations(void);
extern void secure_destroy(void);
@@ -109,6 +96,27 @@ extern ssize_t secure_write(Port *port, void *ptr, size_t len);
extern ssize_t secure_raw_read(Port *port, void *ptr, size_t len);
extern ssize_t secure_raw_write(Port *port, const void *ptr, size_t len);
+/*
+ * declarations for variables defined in be-secure.c
+ */
+extern PGDLLIMPORT char *ssl_library;
+extern PGDLLIMPORT char *ssl_ca_file;
+extern PGDLLIMPORT char *ssl_cert_file;
+extern PGDLLIMPORT char *ssl_crl_file;
+extern PGDLLIMPORT char *ssl_crl_dir;
+extern PGDLLIMPORT char *ssl_key_file;
+extern PGDLLIMPORT int ssl_min_protocol_version;
+extern PGDLLIMPORT int ssl_max_protocol_version;
+extern PGDLLIMPORT char *ssl_passphrase_command;
+extern PGDLLIMPORT bool ssl_passphrase_command_supports_reload;
+extern PGDLLIMPORT char *ssl_dh_params_file;
+extern PGDLLIMPORT char *SSLCipherSuites;
+extern PGDLLIMPORT char *SSLECDHCurve;
+extern PGDLLIMPORT bool SSLPreferServerCiphers;
+#ifdef USE_SSL
+extern PGDLLIMPORT bool ssl_loaded_verify_locations;
+#endif
+
/*
* prototypes for functions in be-secure-gssapi.c
*/
@@ -116,13 +124,6 @@ extern ssize_t secure_raw_write(Port *port, const void *ptr, size_t len);
extern ssize_t secure_open_gssapi(Port *port);
#endif
-/* 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;
-
enum ssl_protocol_versions
{
PG_TLS_ANY = 0,
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?"