Replace px_memset() with explicit_bzero()
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253542psql -h localhost -U postgresBuilt from patchset v1 (message #1), August 25, 2026 at 08:26 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253542_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253542_1 && git checkout t253542_1Patchset v1 (message #1) is on t253542_1
The former was trying to do the same as the latter, but the latter has
more robust and portable implementations, and it's the standard across
the tree, so replace and remove px_memset().
On 25 Aug 2026, at 10:14, Peter Eisentraut <peter@eisentraut.org> wrote:
The former was trying to do the same as the latter, but the latter has more robust and portable implementations, and it's the standard across the tree, so replace and remove px_memset().
While I didn't read the patch, +1 on the concept. It's been on my TODO since
it was reported in /messages/by-id/87ldew2yqu.fsf@wibble.ilmari.org but
hadn't bubbled to the top.
--
Daniel Gustafsson
I eyeballed the patch. Looks mechanical and fine.
One thing stood out, but it's from the existing code:
diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c
index 8191ba02b23..eab86f8206c 100644
--- a/contrib/pgcrypto/crypt-sha.c
+++ b/contrib/pgcrypto/crypt-sha.c
@@ -477,7 +477,7 @@ px_crypt_shacrypt(const char *pw, const char *salt,
char *passwd, unsigned dstle
memcpy(cp, sha_buf_tmp, block); /* Make sure we don't leave something important behind */
- px_memset(&sha_buf_tmp, 0, sizeof sha_buf);
+ explicit_bzero(&sha_buf_tmp, sizeof sha_buf); /*-
* 21. Repeat a loop according to the number specified in the rounds=<N>
That's sha_buf in the sizeof but we're zeroing out sha_buf_tmp.
They're both of length PX_SHACRYPT_DIGEST_MAX_LEN so the result is the
same. It just reads weird.
Might as well fix that now too if going to touch that line.
Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
Sehrope Sarkuni <sehrope@jackdb.com> writes:
One thing stood out, but it's from the existing code: - px_memset(&sha_buf_tmp, 0, sizeof sha_buf); + explicit_bzero(&sha_buf_tmp, sizeof sha_buf);
That's sha_buf in the sizeof but we're zeroing out sha_buf_tmp.
+1 for fixing that. Also I think project style is to write sizeof
with parens.
regards, tom lane
+1 for fixing that. Also I think project style is to write sizeof
with parens.
For as long as I can remember, I have written it with parens when applied
to a type, but without parens when applied to a variable. That's long enough
that I had forgotten why, but Gemini reassures me there's not only a basis
in the language spec (the parens are mandatory when applied to a type) but
also a widely-practiced convention to visibly distinguish the two cases.
Perhaps supporting the idea of a project style that differs, many of
the first hits in our tree from
grep -r '\<sizeof[[:space:]]*[^([:space:]]' src
turn up in imported code like pg_bsd_indent and zic. But I do also turn up
35 matching lines in src/{backend,interfaces}, so there is some precedent
within the project.
Regards,
-Chap