Remove unnecessary static specifier

Started by Japin Li11 months ago6 messages
#1Japin Li
japinli@hotmail.com
1 attachment(s)

Hi,

When reviewing patch [1], I find that the static class specifier is unnecessary
for the variables sp and ep in the function px_crypt_md5().

Attachments:

remove-unnecessary-static-specifier.patchtext/x-diffDownload
diff --git a/contrib/pgcrypto/crypt-md5.c b/contrib/pgcrypto/crypt-md5.c
index d38721a1010..3d17b2340fe 100644
--- a/contrib/pgcrypto/crypt-md5.c
+++ b/contrib/pgcrypto/crypt-md5.c
@@ -36,8 +36,8 @@ px_crypt_md5(const char *pw, const char *salt, char *passwd, unsigned dstlen)
 	static char *magic = "$1$"; /* This string is magic for this algorithm.
 								 * Having it this way, we can get better later
 								 * on */
-	static char *p;
-	static const char *sp,
+	char *p;
+	const char *sp,
 			   *ep;
 	unsigned char final[MD5_SIZE];
 	int			sl,
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Japin Li (#1)
Re: Remove unnecessary static specifier

On 5 Feb 2025, at 13:30, Japin Li <japinli@hotmail.com> wrote:

When reviewing patch [1], I find that the static class specifier is unnecessary
for the variables sp and ep in the function px_crypt_md5().

From a quick first inspection (and running the tests with the patch applied) I
agree with this, these variables do not need to be static. I'll stare a bit
more at this to make sure but seems like the right patch.

--
Daniel Gustafsson

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#2)
Re: Remove unnecessary static specifier

Daniel Gustafsson <daniel@yesql.se> writes:

From a quick first inspection (and running the tests with the patch applied) I
agree with this, these variables do not need to be static. I'll stare a bit
more at this to make sure but seems like the right patch.

+1. All three of those variables are visibly assigned to before any
other reference, so they cannot carry data across calls of the
function.

While we're at it, could we make the adjacent "magic" string be
"static const char *magic"? (Probably needs a couple more
"const" modifiers at use sites, too.)

regards, tom lane

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#3)
Re: Remove unnecessary static specifier

On 5 Feb 2025, at 17:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

While we're at it, could we make the adjacent "magic" string be
"static const char *magic"? (Probably needs a couple more
"const" modifiers at use sites, too.)

Good point, from the link referenced it's clear that FreeBSD has made that
change as well. I'll fix that at the same time.

--
Daniel Gustafsson

#5Japin Li
japinli@hotmail.com
In reply to: Daniel Gustafsson (#4)
Re: Remove unnecessary static specifier

On Wed, 05 Feb 2025 at 18:17, Daniel Gustafsson <daniel@yesql.se> wrote:

On 5 Feb 2025, at 17:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

While we're at it, could we make the adjacent "magic" string be
"static const char *magic"? (Probably needs a couple more
"const" modifiers at use sites, too.)

Good point, from the link referenced it's clear that FreeBSD has made that
change as well. I'll fix that at the same time.

Oh, I didn't notice this. +1.

--
Regrads,
Japin Li

#6Daniel Gustafsson
daniel@yesql.se
In reply to: Japin Li (#5)
Re: Remove unnecessary static specifier

On 6 Feb 2025, at 02:51, Japin Li <japinli@hotmail.com> wrote:

On Wed, 05 Feb 2025 at 18:17, Daniel Gustafsson <daniel@yesql.se> wrote:

On 5 Feb 2025, at 17:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

While we're at it, could we make the adjacent "magic" string be
"static const char *magic"? (Probably needs a couple more
"const" modifiers at use sites, too.)

Good point, from the link referenced it's clear that FreeBSD has made that
change as well. I'll fix that at the same time.

Oh, I didn't notice this. +1.

Committed, thanks!

--
Daniel Gustafsson