Remaining reference to _PG_fini() in ldap_password_func

Started by Michael Paquierover 1 year ago5 messages
#1Michael Paquier
michael@paquier.xyz
1 attachment(s)

Hi all,

While hacking on a different thing, I've noticed that
ldap_password_func had the idea to define _PG_fini(). This is
pointless, as library unloading is not supported in the backend, and
something that has been cleaned up from the tree in ab02d702ef08.
That's not something to encourage, perhaps, as well..

How about removing it like in the attached to be consistent?

Thanks,
--
Michael

Attachments:

ldap-module-pgfini.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/test/modules/ldap_password_func/ldap_password_func.c b/src/test/modules/ldap_password_func/ldap_password_func.c
index 99c18a8f1c..24d9c63781 100644
--- a/src/test/modules/ldap_password_func/ldap_password_func.c
+++ b/src/test/modules/ldap_password_func/ldap_password_func.c
@@ -23,7 +23,6 @@
 PG_MODULE_MAGIC;
 
 void		_PG_init(void);
-void		_PG_fini(void);
 
 /* hook function */
 static char *rot13_passphrase(char *password);
@@ -37,12 +36,6 @@ _PG_init(void)
 	ldap_password_hook = rot13_passphrase;
 }
 
-void
-_PG_fini(void)
-{
-	/* do  nothing yet */
-}
-
 static char *
 rot13_passphrase(char *pw)
 {
#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Paquier (#1)
Re: Remaining reference to _PG_fini() in ldap_password_func

On 20/08/2024 07:46, Michael Paquier wrote:

Hi all,

While hacking on a different thing, I've noticed that
ldap_password_func had the idea to define _PG_fini(). This is
pointless, as library unloading is not supported in the backend, and
something that has been cleaned up from the tree in ab02d702ef08.
That's not something to encourage, perhaps, as well..

How about removing it like in the attached to be consistent?

+1. There's also a prototype for _PG_fini() in fmgr.h, let's remove that
too.

--
Heikki Linnakangas
Neon (https://neon.tech)

#3Michael Paquier
michael@paquier.xyz
In reply to: Heikki Linnakangas (#2)
Re: Remaining reference to _PG_fini() in ldap_password_func

On Tue, Aug 20, 2024 at 09:59:12AM +0300, Heikki Linnakangas wrote:

+1. There's also a prototype for _PG_fini() in fmgr.h, let's remove that
too.

Yes, I was wondering too whether we should force the hand of
extensions to know that it is pointless to support unloading, telling
everybody to delete this code.

I'm OK with the agressive move and remove the whole. Any thoughts
from others?
--
Michael

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#2)
Re: Remaining reference to _PG_fini() in ldap_password_func

Heikki Linnakangas <hlinnaka@iki.fi> writes:

On 20/08/2024 07:46, Michael Paquier wrote:

How about removing it like in the attached to be consistent?

+1. There's also a prototype for _PG_fini() in fmgr.h, let's remove that
too.

+1. I think the fmgr.h prototype may have been left there
deliberately to avoid breaking extension code, but it's past
time to clean it up.

regards, tom lane

#5Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#4)
Re: Remaining reference to _PG_fini() in ldap_password_func

On Tue, Aug 20, 2024 at 09:05:54AM -0400, Tom Lane wrote:

Heikki Linnakangas <hlinnaka@iki.fi> writes:

+1. There's also a prototype for _PG_fini() in fmgr.h, let's remove that
too.

+1. I think the fmgr.h prototype may have been left there
deliberately to avoid breaking extension code, but it's past
time to clean it up.

Okay, I've removed all that then. Thanks for the feedback.
--
Michael