On Fri, Oct 31, 2025 at 1:31 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
>
> On Thu, Oct 23, 2025 at 2:48 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > I've drafted the patches for this item.
>
> Thanks!
>
> > The 0001 patch allows the packager to select the random source:
> > "openssl" or "system", by using --with-random-source option. If it's
> > omitted and OpenSSL is used (--with-openssl or --with-ssl=openssl),
> > 'openssl' source is automatically chosen. The selected random source
> > can be shown in read-only GUC parameter random_source.
Thank you for the comments.
>
> I think 0001 is missing logic to switch pg_strong_random.c according
> to the new #defines; selecting `system` still uses OpenSSL on my
> machine.
Fixed.
>
> > + --with-random-source=NAME
> > + set random number source (system,openssl)
>
> Bikeshedding: should we make it clear in the name that this covers
> only the "strong" random implementation, for cryptography? Maybe
> --with-strong-random, or --with-crypt-random, or...
Agreed, I renamed it to --with-strong-random and applied it to other
related names too .
>
> > + AC_DEFINE([USE_RANDOM_SOURCE_OPENSSL] , 1, [Define to 1 to use OpenSSL libary for random number generation])
>
> autoreconf 2.69 is refusing to process these AC_DEFINEs on my machine,
> and I think it's because of the space between the end bracket ']' and
> the comma. Removing it fixes generation for me.
Fixed.
>
> > @@ -3500,5 +3507,4 @@
> > options => 'io_method_options',
> > assign_hook => 'assign_io_method',
> > },
> > -
> > ]
>
> (FWIW, I prefer the extra newline there; it's easier for me to read
> and it matches the spacing at the top.)
Fixed.
>
> > The 0002 patch supports getrandom() as a 'system' random source where
> > available while keeping the method of reading /dev/urandom as a
> > fallback option.
>
> > +AC_CHECK_HEADER([sys/random.h],
> > + [AC_CHECK_FUNCS([getrandom],
> > + [AC_DEFINE(HAVE_GETRANDOM, 1, [Define to 1 if you have getrandom])])])
>
> AC_CHECK_FUNCS([getrandom]) by itself should let autoconf take care of
> the description; I think the extra AC_DEFINE there will duplicate some
> handling.
>
> > +if cc.has_header('sys/random.h') and cc.has_function('getrandom',
> > + args: test_c_args, prefix: '''
> > +#include <sys/random.h>''')
> > + cdata.set('HAVE_GETRANDOM', 1)
> > +endif
>
> Some of the more complicated prefixes use multiline literals, but for
> this case I think `prefix: '#include <sys/random.h'` would be more
> readable. Also, is there a way to merge this check into the
> check_funcs logic later on?
Agreed with the above comments. I think we don't need a header check
for sys/random.h actually. Checking the getrandom() function present
seems to be sufficient in this case.
I've attached the updated patches.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
