pgcrypto: remove useless px_memset() and BF_ASM

Started by Dagfinn Ilmari Mannsåker5 months ago2 messageshackers
Beta feature

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.

won't retrysuccessCI history

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:t139267
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 27, 2026 at 01:57 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 t139267_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t139267_1 && git checkout t139267_1

Patchset v1 (message #1) is on t139267_1

Jump to latest

Hi Hackers,

In the thread about centralised architecture detection, I noticed that
the BF_ASM macro in crypt-blowfish.c has never been defined to anything
but 0, and the _BF_body_r() function it would call has never existed, so
that can be got rid of.

While investigating at that, I also noticed that px_memset(), which has
the comment /* memset that must not be optimized away */, is only ever
called with zero for the value, which could be better written with
explicit_bzero() now that we have that.

Attached are patches for both.

- ilmari

Attachments:

t139267_1
0001-pgcrypto-remove-unused-BF_ASM-macro.patchtext/x-diffDownload+0-13
0002-pgcrypto-use-explicit_bzero-instead-of-px_memset-.-0.patchtext/x-diffDownload+49-59
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: pgcrypto: remove useless px_memset() and BF_ASM

On 9 Apr 2026, at 13:51, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

In the thread about centralised architecture detection, I noticed that
the BF_ASM macro in crypt-blowfish.c has never been defined to anything
but 0, and the _BF_body_r() function it would call has never existed, so
that can be got rid of.

Agreed. I didn't do enough archaeology to figure out what upstream has/had or
why it was removed, but it's been dead for 25 odd years so it's about time to
remove.

While investigating at that, I also noticed that px_memset(), which has
the comment /* memset that must not be optimized away */, is only ever
called with zero for the value, which could be better written with
explicit_bzero() now that we have that.

One could imagine various tricks for rewriting px_memset to explicit_bzero in
order to reduce the churn, but since this code is very rarely backpatched into
it's not a big problem IMHO.

I'll have another look with fresh eyes once back in the office.

--
Daniel Gustafsson