Postgres Pro
Downloads
Home   >   mailing lists

Thread: wrong comment in libpq.h

wrong comment in libpq.h

From
David Zhang
Date:
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
Attachment

Re: wrong comment in libpq.h

From
Peter Eisentraut
Date:
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.

Re: wrong comment in libpq.h

From
Daniel Gustafsson
Date:
> 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

Re: wrong comment in libpq.h

From
David Zhang
Date:
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

Re: wrong comment in libpq.h

From
Peter Eisentraut
Date:
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.

Re: wrong comment in libpq.h

From
David Zhang
Date:
> 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

Re: wrong comment in libpq.h

From
Daniel Gustafsson
Date:
> 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

Re: wrong comment in libpq.h

From
Bruce Momjian
Date:
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?"